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.

Sign up for GitHub

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

Jump to bottom

Fix missing Discovery.Azure IExtension #2653

Merged
merged 5 commits into from
Jul 15, 2024
Merged

Fix missing Discovery.Azure IExtension #2653

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
36406f0
Fix missing Discovery.Azure IExtension
Arkatufus Jul 15, 2024
6626e73
Cleanup code
Arkatufus Jul 15, 2024
cdfb86b
Merge branch 'dev' into fix-missing-Azure-IExtension
Arkatufus Jul 15, 2024
603b0d8
Merge branch 'dev' into fix-missing-Azure-IExtension
Arkatufus Jul 15, 2024
54260c6
Merge branch 'dev' into fix-missing-Azure-IExtension
Aaronontheweb Jul 15, 2024
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
6 changes: 3 additions & 3 deletions src/discovery/azure/Akka.Discovery.Azure.Tests/AzureDiscoverySettingsSpecs.cs
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/discovery/azure/Akka.Discovery.Azure/AkkaDiscoveryOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,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
8 changes: 6 additions & 2 deletions src/discovery/azure/Akka.Discovery.Azure/AkkaHostingExtensions.cs
View file
Open in desktop
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
View file
Open in desktop
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);
}
4 changes: 1 addition & 3 deletions src/discovery/azure/Akka.Discovery.Azure/AzureServiceDiscovery.cs
View file
Open in desktop
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