Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing Discovery.Azure IExtension #2653

Merged
merged 5 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ public class AzureDiscoverySettingsSpecs
[Fact(DisplayName = "Default settings should contain default values")]
public void DefaultSettingsTest()
{
var settings = AzureDiscoverySettings.Create(AzureServiceDiscovery.DefaultConfig);
var settings = AzureDiscoverySettings.Create(AzureDiscovery.DefaultConfiguration());

var assemblyName = typeof(AzureServiceDiscovery).Assembly.FullName!.Split(',')[0].Trim();
var config = AzureServiceDiscovery.DefaultConfig.GetConfig("akka.discovery.azure");
var config = AzureDiscovery.DefaultConfiguration().GetConfig(AzureServiceDiscovery.DefaultConfigPath);
config.GetString("class").Should().Be($"{typeof(AzureServiceDiscovery).Namespace}.{nameof(AzureServiceDiscovery)}, {assemblyName}");

settings.ReadOnly.Should().BeFalse();
Expand All @@ -44,7 +44,7 @@ public void DefaultSettingsTest()
[Fact(DisplayName = "Empty settings variable and default settings should match")]
public void EmptySettingsTest()
{
var settings = AzureDiscoverySettings.Create(AzureServiceDiscovery.DefaultConfig);
var settings = AzureDiscoverySettings.Create(AzureDiscovery.DefaultConfiguration());
var empty = AzureDiscoverySettings.Empty;

empty.ReadOnly.Should().Be(settings.ReadOnly);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public void Apply(AkkaConfigurationBuilder builder, Setup? inputSetup = null)

builder.AddHocon(sb.ToString(), HoconAddMode.Prepend);

var fallback = AzureServiceDiscovery.DefaultConfig
var fallback = AzureDiscovery.DefaultConfiguration()
.GetConfig(AzureServiceDiscovery.FullPath(AzureServiceDiscovery.DefaultPath))
.MoveTo(AzureServiceDiscovery.FullPath(ConfigPath));
builder.AddHocon(fallback, HoconAddMode.Append);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,12 @@ public static AkkaConfigurationBuilder WithAzureDiscovery(
AkkaDiscoveryOptions options)
{
options.Apply(builder);

builder.AddHocon(AzureServiceDiscovery.DefaultConfig, HoconAddMode.Append);

// force start the module
builder.AddStartup((system, registry) =>
{
AzureDiscovery.Get(system);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary now but not before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before, the discovery module was automatically started by ClusterBootstrap during startup. Since ClusterBootstrap might not run in tandem with AzureDiscovery anymore, we need to force start it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, makes sense

});
return builder;
}
}
Expand Down
31 changes: 31 additions & 0 deletions src/discovery/azure/Akka.Discovery.Azure/AzureDiscovery.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using Akka.Actor;
using Akka.Configuration;

namespace Akka.Discovery.Azure;

public class AzureDiscovery: IExtension
{
public static Configuration.Config DefaultConfiguration()
=> ConfigurationFactory.FromResource<AzureDiscovery>("Akka.Discovery.Azure.reference.conf");

public static AzureDiscovery Get(ActorSystem system)
=> system.WithExtension<AzureDiscovery, AzureDiscoveryProvider>();

public readonly AzureDiscoverySettings Settings;

public AzureDiscovery(ExtendedActorSystem system)
{
system.Settings.InjectTopLevelFallback(DefaultConfiguration());
Settings = AzureDiscoverySettings.Create(system);

var setup = system.Settings.Setup.Get<AzureDiscoverySetup>();
if (setup.HasValue)
Settings = setup.Value.Apply(Settings);
}
}

public class AzureDiscoveryProvider : ExtensionIdProvider<AzureDiscovery>
{
public override AzureDiscovery CreateExtension(ExtendedActorSystem system)
=> new AzureDiscovery(system);
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ public class AzureServiceDiscovery : ServiceDiscovery
internal const string DefaultPath = "azure";
internal const string DefaultConfigPath = "akka.discovery." + DefaultPath;
internal static string FullPath(string path) => $"akka.discovery.{path}";
public static readonly Configuration.Config DefaultConfig =
ConfigurationFactory.FromResource<AzureServiceDiscovery>("Akka.Discovery.Azure.reference.conf");

private readonly ILoggingAdapter _log;
private readonly ExtendedActorSystem _system;
Expand All @@ -42,7 +40,7 @@ public AzureServiceDiscovery(ExtendedActorSystem system, Configuration.Config co
_system = system;
_log = Logging.GetLogger(system, typeof(AzureServiceDiscovery));

var fullConfig = config.WithFallback(DefaultConfig.GetConfig(DefaultConfigPath));
var fullConfig = config.WithFallback(AzureDiscovery.DefaultConfiguration().GetConfig(DefaultConfigPath));
_settings = AzureDiscoverySettings.Create(system, fullConfig);

var setup = _system.Settings.Setup.Get<AzureDiscoverySetup>();
Expand Down