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

[Discovery.KubernetesApi] add option to query pods in all namespaces #2421

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
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public void DefaultSettingsTest()
settings.ApiServicePortEnvName.Should().Be("KUBERNETES_SERVICE_PORT");
settings.PodNamespacePath.Should().Be("/var/run/secrets/kubernetes.io/serviceaccount/namespace");
settings.PodNamespace.Should().BeNull();
settings.AllNamespaces.Should().BeFalse();
settings.PodDomain.Should().Be("cluster.local");
settings.PodLabelSelector("a").Should().Be("app=a");
settings.RawIp.Should().BeTrue();
Expand All @@ -47,6 +48,7 @@ public void EmptySettingsTest()
empty.ApiServicePortEnvName.Should().Be(settings.ApiServicePortEnvName);
empty.PodNamespacePath.Should().Be(settings.PodNamespacePath);
empty.PodNamespace.Should().Be(settings.PodNamespace);
empty.AllNamespaces.Should().Be(settings.AllNamespaces);
empty.PodDomain.Should().Be(settings.PodDomain);
empty.PodLabelSelector("a").Should().Be(settings.PodLabelSelector("a"));
empty.RawIp.Should().Be(settings.RawIp);
Expand All @@ -63,6 +65,7 @@ public void WithOverrideTest()
.WithApiServicePortEnvName("d")
.WithPodNamespacePath("e")
.WithPodNamespace("f")
.WithAllNamespaces(true)
.WithPodDomain("g")
.WithPodLabelSelector("h={0}")
.WithRawIp(false)
Expand All @@ -74,6 +77,7 @@ public void WithOverrideTest()
settings.ApiServicePortEnvName.Should().Be("d");
settings.PodNamespacePath.Should().Be("e");
settings.PodNamespace.Should().Be("f");
settings.AllNamespaces.Should().BeTrue();
settings.PodDomain.Should().Be("g");
settings.PodLabelSelector("a").Should().Be("h=a");
settings.RawIp.Should().BeFalse();
Expand All @@ -91,6 +95,7 @@ public void SetupOverrideTest()
ApiServicePortEnvName = "d",
PodNamespacePath = "e",
PodNamespace = "f",
AllNamespaces = true,
PodDomain = "g",
PodLabelSelector = "h={0}",
RawIp = false,
Expand All @@ -104,6 +109,7 @@ public void SetupOverrideTest()
settings.ApiServicePortEnvName.Should().Be("d");
settings.PodNamespacePath.Should().Be("e");
settings.PodNamespace.Should().Be("f");
settings.AllNamespaces.Should().BeTrue();
settings.PodDomain.Should().Be("g");
settings.PodLabelSelector("a").Should().Be("h=a");
settings.RawIp.Should().BeFalse();
Expand All @@ -122,6 +128,7 @@ public void ModuleSetupTest()
ApiServicePortEnvName = "d",
PodNamespacePath = "e",
PodNamespace = "f",
AllNamespaces = true,
PodDomain = "g",
PodLabelSelector = "h={0}",
RawIp = false,
Expand All @@ -138,6 +145,7 @@ public void ModuleSetupTest()
settings.ApiServicePortEnvName.Should().Be("d");
settings.PodNamespacePath.Should().Be("e");
settings.PodNamespace.Should().Be("f");
settings.AllNamespaces.Should().BeTrue();
settings.PodDomain.Should().Be("g");
settings.PodLabelSelector("a").Should().Be("h=a");
settings.RawIp.Should().BeFalse();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,27 +89,17 @@ public override async Task<Resolved> Lookup(Lookup lookup, TimeSpan resolveTimeo
var labelSelector = _settings.PodLabelSelector(lookup.ServiceName);

if(_log.IsInfoEnabled)
_log.Info("Querying for pods with label selector: [{0}]. Namespace: [{1}]. Port: [{2}]",
labelSelector, PodNamespace, lookup.PortName);
_log.Info("Querying for pods with label selector: [{0}]. Namespace: [{1}]. AllNamespaces: [{2}]. Port: [{3}]",
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

labelSelector, PodNamespace, _settings.AllNamespaces, lookup.PortName);

var cts = new CancellationTokenSource(resolveTimeout);
V1PodList podList;
try
{
#if !NET6_0_OR_GREATER
Copy link
Member

Choose a reason for hiding this comment

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

So we moved the ifdefs down into these individual methods - got it

var result = await _client.ListNamespacedPodWithHttpMessagesAsync(
namespaceParameter: PodNamespace,
labelSelector: labelSelector,
cancellationToken: cts.Token)
.ConfigureAwait(false);
podList = result.Body;
#else
var result = await _client.ListNamespacedPodAsync(
namespaceParameter: PodNamespace,
labelSelector: labelSelector,
cancellationToken: cts.Token);
podList = result;
#endif
if(_settings.AllNamespaces)
podList = await ListPodForAllNamespaces(labelSelector, cts);
else
podList = await ListNamespacedPod(labelSelector, cts);
}
catch (SerializationException e)
{
Expand Down Expand Up @@ -177,7 +167,45 @@ public override async Task<Resolved> Lookup(Lookup lookup, TimeSpan resolveTimeo

return new Resolved(serviceName: lookup.ServiceName, addresses: addresses);
}

private async Task<V1PodList> ListNamespacedPod(string labelSelector, CancellationTokenSource cts)
Copy link
Member

Choose a reason for hiding this comment

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

LGTM - basically what we had before

{
V1PodList podList;
#if !NET6_0_OR_GREATER
var result = await _client.ListNamespacedPodWithHttpMessagesAsync(
namespaceParameter: PodNamespace,
labelSelector: labelSelector,
cancellationToken: cts.Token)
.ConfigureAwait(false);
podList = result.Body;
#else
var result = await _client.ListNamespacedPodAsync(
namespaceParameter: PodNamespace,
labelSelector: labelSelector,
cancellationToken: cts.Token);
podList = result;
#endif
return podList;
}

private async Task<V1PodList> ListPodForAllNamespaces(string labelSelector, CancellationTokenSource cts)
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

{
V1PodList podList;
#if !NET6_0_OR_GREATER
var result = await _client.ListPodForAllNamespacesWithHttpMessagesAsync(
labelSelector: labelSelector,
cancellationToken: cts.Token)
.ConfigureAwait(false);
podList = result.Body;
#else
var result = await _client.ListPodForAllNamespacesAsync(
labelSelector: labelSelector,
cancellationToken: cts.Token);
podList = result;
#endif
return podList;
}

// This uses blocking IO, and so should only be used to read configuration at startup.
private string? ReadConfigVarFromFileSystem(string path, string name)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class KubernetesDiscoveryOptions: IHoconOption
public string? ApiServicePortEnvName { get; set; }
public string? PodNamespacePath { get; set; }
public string? PodNamespace { get; set; }
public bool? AllNamespaces { get; set; }
public string? PodDomain { get; set; }
public string? PodLabelSelector { get; set; }
public bool? RawIp { get; set; }
Expand All @@ -49,6 +50,8 @@ public void Apply(AkkaConfigurationBuilder builder, Setup? setup = null)
sb.AppendLine($"pod-namespace-path = {PodNamespacePath.ToHocon()}");
if (PodNamespace is { })
sb.AppendLine($"pod-namespace = {PodNamespace.ToHocon()}");
if (AllNamespaces is { })
sb.AppendLine($"all-namespaces = {AllNamespaces.ToHocon()}");
if (PodDomain is { })
sb.AppendLine($"pod-domain = {PodDomain.ToHocon()}");
if (PodLabelSelector is { })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public static KubernetesDiscoverySettings Create(Configuration.Config config)
config.GetString("api-service-port-env-name"),
config.GetString("pod-namespace-path"),
config.GetStringIfDefined("pod-namespace"),
config.GetBoolean("all-namespaces"),
config.GetString("pod-domain"),
config.GetString("pod-label-selector"),
config.GetBoolean("use-raw-ip"),
Expand All @@ -40,6 +41,7 @@ private KubernetesDiscoverySettings(
string apiServicePortEnvName,
string podNamespacePath,
string? podNamespace,
bool allNamespaces,
string podDomain,
string podLabelSelector,
bool rawIp,
Expand All @@ -51,6 +53,7 @@ private KubernetesDiscoverySettings(
ApiServicePortEnvName = apiServicePortEnvName;
PodNamespacePath = podNamespacePath;
PodNamespace = podNamespace;
AllNamespaces = allNamespaces;
PodDomain = podDomain;
_podLabelSelector = podLabelSelector;
RawIp = rawIp;
Expand All @@ -63,6 +66,7 @@ private KubernetesDiscoverySettings(
public string ApiServicePortEnvName { get; }
public string PodNamespacePath { get; }
public string? PodNamespace { get; }
public bool AllNamespaces { get; }
public string PodDomain { get; }
public string PodLabelSelector(string name)
=> string.Format(_podLabelSelector, name);
Expand All @@ -81,6 +85,8 @@ public KubernetesDiscoverySettings WithPodNamespacePath(string podNamespacePath)
=> Copy(podNamespacePath: podNamespacePath);
public KubernetesDiscoverySettings WithPodNamespace(string podNamespace)
=> Copy(podNamespace: podNamespace);
public KubernetesDiscoverySettings WithAllNamespaces(bool allNamespaces)
=> Copy(allNamespaces: allNamespaces);
public KubernetesDiscoverySettings WithPodDomain(string podDomain)
=> Copy(podDomain: podDomain);
public KubernetesDiscoverySettings WithPodLabelSelector(string podLabelSelector)
Expand All @@ -97,6 +103,7 @@ internal KubernetesDiscoverySettings Copy(
string? apiServicePortEnvName = null,
string? podNamespacePath = null,
string? podNamespace = null,
bool? allNamespaces = null,
string? podDomain = null,
string? podLabelSelector = null,
bool? rawIp = null,
Expand All @@ -108,6 +115,7 @@ internal KubernetesDiscoverySettings Copy(
apiServicePortEnvName: apiServicePortEnvName ?? ApiServicePortEnvName,
podNamespacePath: podNamespacePath ?? PodNamespacePath,
podNamespace: podNamespace ?? PodNamespace,
allNamespaces: allNamespaces ?? AllNamespaces,
podDomain: podDomain ?? PodDomain,
podLabelSelector: podLabelSelector ?? _podLabelSelector,
rawIp: rawIp ?? RawIp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public class KubernetesDiscoverySetup: Setup
public string? ApiServicePortEnvName { get; set; }
public string? PodNamespacePath { get; set; }
public string? PodNamespace { get; set; }
public bool? AllNamespaces { get; set; }
public string? PodDomain { get; set; }
public string? PodLabelSelector { get; set; }
public bool? RawIp { get; set; }
Expand All @@ -31,6 +32,7 @@ internal KubernetesDiscoverySettings Apply(KubernetesDiscoverySettings settings)
apiServicePortEnvName: ApiServicePortEnvName,
podNamespacePath: PodNamespacePath,
podNamespace: PodNamespace,
allNamespaces: AllNamespaces,
podDomain: PodDomain,
podLabelSelector: PodLabelSelector,
rawIp: RawIp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ akka.discovery.kubernetes-api {
# Set this value to a specific string to override discovering the namespace using pod-namespace-path.
pod-namespace = "<pod-namespace>"

# Enable to query pods in all namespaces
#
# If this is set to true, the pod-namespace configuration is ignored.
all-namespaces = true
Copy link
Member

Choose a reason for hiding this comment

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

LGTM


# Domain of the k8s cluster
pod-domain = "cluster.local"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ akka.discovery {
#
# Set this value to a specific string to override discovering the namespace using pod-namespace-path.
pod-namespace = "<pod-namespace>"


# Enable to query pods in all namespaces
#
# If this is set to true, the pod-namespace configuration is ignored.
all-namespaces = false

# Domain of the k8s cluster
pod-domain = "cluster.local"

Expand Down