Skip to content

Commit

Permalink
Ensure k8s reregistration is correctly (#802)
Browse files Browse the repository at this point in the history 
* add tests

* add implementation

* Make it anonymous

* Make it obvious why we are checking if id != null
  • Loading branch information
@scme0
scme0 authored Nov 16, 2023
1 parent 60828a2 commit 22026c2
Show file tree
Hide file tree
Showing 3 changed files with 206 additions and 24 deletions.
149 changes: 149 additions & 0 deletions source/Octopus.Client.Tests/Operations/RegisterKubernetesClusterOperationFixture.cs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using FluentAssertions;
using NSubstitute;
using NUnit.Framework;
using Octopus.Client.Exceptions;
using Octopus.Client.Extensibility;
using Octopus.Client.Model;
using Octopus.Client.Model.Endpoints;
Expand All @@ -22,6 +24,7 @@ public class RegisterKubernetesClusterOperationFixture
ResourceCollection<EnvironmentResource> environments;
ResourceCollection<MachineResource> machines;
ResourceCollection<MachinePolicyResource> machinePolicies;
private ResourceCollection<ProxyResource> proxies;

[SetUp]
public void SetUp()
Expand All @@ -32,6 +35,7 @@ public void SetUp()
operation = new RegisterKubernetesClusterOperation(clientFactory);
serverEndpoint = new OctopusServerEndpoint("http://octopus", "ABC123");

proxies = new ResourceCollection<ProxyResource>(new ProxyResource[0], LinkCollection.Self("/foo"));
environments = new ResourceCollection<EnvironmentResource>(new EnvironmentResource[0], LinkCollection.Self("/foo"));
machines = new ResourceCollection<MachineResource>(new MachineResource[0], LinkCollection.Self("/foo"));
machinePolicies = new ResourceCollection<MachinePolicyResource>(new MachinePolicyResource[0], LinkCollection.Self("/foo"));
Expand All @@ -45,6 +49,7 @@ public void SetUp()
.Add("MachinePolicies", "/api/machinepolicies")
.Add("CurrentUser", "/api/users/me")
.Add("SpaceHome", "/api/spaces")
.Add("Proxies", "/api/proxies")
};

client.Get<RootResource>(Arg.Any<string>(), Arg.Any<CancellationToken>()).Returns(rootDocument);
Expand All @@ -57,6 +62,8 @@ public void SetUp()
.Do(ci => ci.Arg<Func<ResourceCollection<MachineResource>, bool>>()(machines));
client.When(x => x.Paginate(Arg.Any<string>(), Arg.Any<object>(), Arg.Any<Func<ResourceCollection<MachinePolicyResource>, bool>>(), Arg.Any<CancellationToken>()))
.Do(ci => ci.Arg<Func<ResourceCollection<MachinePolicyResource>, bool>>()(machinePolicies));
client.When(x => x.Paginate(Arg.Any<string>(), Arg.Any<object>(), Arg.Any<Func<ResourceCollection<ProxyResource>, bool>>(), Arg.Any<CancellationToken>()))
.Do(ci => ci.Arg<Func<ResourceCollection<ProxyResource>, bool>>()(proxies));
}

[Test]
Expand Down Expand Up @@ -101,5 +108,147 @@ public async Task ShouldCreateNewPollingKubernetesTentacle()

machineResources.Should().ContainSingle().Which.Endpoint.Should().BeOfType<KubernetesTentacleEndpointResource>().Which.Should().BeEquivalentTo(new KubernetesTentacleEndpointResource(new PollingTentacleEndpointConfigurationResource("ABCDEF", "poll://ckyhfyfkcbmzjl8sfgch/")));
}

[Test]
public async Task ShouldOnlyUpdateEndpointConnectionConfigurationOnExistingMachine()
{
var updatedMachines = new List<MachineResource>();
await client.Update("/machines/whatever/1", Arg.Do<MachineResource>(m => updatedMachines.Add(m)), Arg.Any<object>(), Arg.Any<CancellationToken>());

environments.Items.Add(new EnvironmentResource {Id = "environments-1", Name = "UAT", Links = LinkCollection.Self("/api/environments/environments-1").Add("Machines", "/api/environments/environments-1/machines")});
environments.Items.Add(new EnvironmentResource {Id = "environments-2", Name = "Production", Links = LinkCollection.Self("/api/environments/environments-2").Add("Machines", "/api/environments/environments-2/machines")});

machines.Items.Add(new MachineResource
{
Id = "machines/84", EnvironmentIds = new ReferenceCollection(new[] { "environments-1" }), Name = "Mymachine", Links = LinkCollection.Self("/machines/whatever/1"),
Endpoint = new KubernetesTentacleEndpointResource(new ListeningTentacleEndpointConfigurationResource("123456", "myMachine.test.com") { ProxyId = "proxy-2" })
});

proxies.Items.Add(new ProxyResource { Id = "proxy-1", Name = "MyNewProxy", Links = LinkCollection.Self("/api/proxies/proxy-1") });

operation.MachineName = "Mymachine";
operation.TentaclePort = 10930;
operation.TentacleThumbprint = "ABCDEF";
operation.TentacleHostname = "my-new-machine.test.com";
operation.CommunicationStyle = CommunicationStyle.TentaclePassive;
operation.EnvironmentNames = new[] {"Production"};
operation.ProxyName = "MyNewProxy";

await operation.ExecuteAsync(serverEndpoint).ConfigureAwait(false);

var thing = updatedMachines.Should().ContainSingle().Which;

thing.Should().BeEquivalentTo(new
{
Id = "machines/84",
Name = "Mymachine",
EnvironmentIds = new ReferenceCollection("environments-1"),
Endpoint = new KubernetesTentacleEndpointResource(new ListeningTentacleEndpointConfigurationResource("ABCDEF", "https://my-new-machine.test.com:10930/") {ProxyId = "proxy-1"}),
Links = LinkCollection.Self("/machines/whatever/1")
});
}

[Test]
public async Task ShouldUpdateExistingMachineWhenForceIsEnabled()
{
environments.Items.Add(new EnvironmentResource {Id = "environments-1", Name = "UAT", Links = LinkCollection.Self("/api/environments/environments-1").Add("Machines", "/api/environments/environments-1/machines")});
environments.Items.Add(new EnvironmentResource {Id = "environments-2", Name = "Production", Links = LinkCollection.Self("/api/environments/environments-2").Add("Machines", "/api/environments/environments-2/machines")});

machines.Items.Add(new MachineResource {Id = "machines/84", EnvironmentIds = new ReferenceCollection(new[] {"environments-1"}), Name = "Mymachine", Links = LinkCollection.Self("/machines/whatever/1")});

operation.TentacleThumbprint = "ABCDEF";
operation.TentaclePort = 10930;
operation.MachineName = "Mymachine";
operation.TentacleHostname = "Mymachine.test.com";
operation.CommunicationStyle = CommunicationStyle.TentaclePassive;
operation.EnvironmentNames = new[] {"Production"};
operation.AllowOverwrite = true;

await operation.ExecuteAsync(serverEndpoint).ConfigureAwait(false);

await client.Received().Update("/machines/whatever/1", Arg.Is<MachineResource>(m =>
m.Id == "machines/84"
&& m.Name == "Mymachine"
&& m.EnvironmentIds.First() == "environments-2"),
Arg.Any<object>(), Arg.Any<CancellationToken>()).ConfigureAwait(false);
}

[Test]
public async Task ShouldCreateWhenCantDeserializeMachines()
{
client.When(x => x.Paginate(Arg.Any<string>(), Arg.Any<object>(), Arg.Any<Func<ResourceCollection<MachineResource>, bool>>()))
.Throw(new OctopusDeserializationException(1, "Can not deserialize"));

environments.Items.Add(new EnvironmentResource { Id = "environments-1", Name = "UAT", Links = LinkCollection.Self("/api/environments/environments-1").Add("Machines", "/api/environments/environments-1/machines") });
environments.Items.Add(new EnvironmentResource { Id = "environments-2", Name = "Production", Links = LinkCollection.Self("/api/environments/environments-2").Add("Machines", "/api/environments/environments-2/machines") });

operation.TentacleThumbprint = "ABCDEF";
operation.TentaclePort = 10930;
operation.MachineName = "Mymachine";
operation.TentacleHostname = "Mymachine.test.com";
operation.CommunicationStyle = CommunicationStyle.TentaclePassive;
operation.EnvironmentNames = new[] { "Production" };

await operation.ExecuteAsync(serverEndpoint).ConfigureAwait(false);

await client.Received().Create("/api/machines", Arg.Is<MachineResource>(m =>
m.Name == "Mymachine"
&& ((KubernetesTentacleEndpointResource)m.Endpoint).TentacleEndpointConfiguration.Uri == "https://mymachine.test.com:10930/"
&& m.EnvironmentIds.First() == "environments-2"),
Arg.Any<object>(), Arg.Any<CancellationToken>()).ConfigureAwait(false);
}

[Test]
public async Task ShouldNotOverwriteMachinePolicyToNull()
{
environments.Items.Add(new EnvironmentResource { Id = "environments-1", Name = "UAT", Links = LinkCollection.Self("/api/environments/environments-1").Add("Machines", "/api/environments/environments-1/machines") });
environments.Items.Add(new EnvironmentResource { Id = "environments-2", Name = "Production", Links = LinkCollection.Self("/api/environments/environments-2").Add("Machines", "/api/environments/environments-2/machines") });

machines.Items.Add(new MachineResource { Id = "machines/84", MachinePolicyId = "MachinePolicies-1", EnvironmentIds = new ReferenceCollection(new[] { "environments-1" }), Name = "Mymachine", Links = LinkCollection.Self("/machines/whatever/1") });

operation.TentacleThumbprint = "ABCDEF";
operation.TentaclePort = 10930;
operation.MachineName = "Mymachine";
operation.TentacleHostname = "Mymachine.test.com";
operation.CommunicationStyle = CommunicationStyle.TentaclePassive;
operation.EnvironmentNames = new[] { "Production" };
operation.AllowOverwrite = true;

await operation.ExecuteAsync(serverEndpoint).ConfigureAwait(false);

await client.Received().Update("/machines/whatever/1", Arg.Is<MachineResource>(m =>
m.Id == "machines/84"
&& m.Name == "Mymachine"
&& m.EnvironmentIds.First() == "environments-2"
&& m.MachinePolicyId == "MachinePolicies-1"),
Arg.Any<object>(), Arg.Any<CancellationToken>()).ConfigureAwait(false);
}

[Test]
public async Task ShouldOverwriteMachinePolicyWhenPassed()
{
environments.Items.Add(new EnvironmentResource { Id = "environments-1", Name = "UAT", Links = LinkCollection.Self("/api/environments/environments-1").Add("Machines", "/api/environments/environments-1/machines") });
environments.Items.Add(new EnvironmentResource { Id = "environments-2", Name = "Production", Links = LinkCollection.Self("/api/environments/environments-2").Add("Machines", "/api/environments/environments-2/machines") });

machines.Items.Add(new MachineResource { Id = "machines/84", MachinePolicyId = "MachinePolicies-1", EnvironmentIds = new ReferenceCollection(new[] { "environments-1" }), Name = "Mymachine", Links = LinkCollection.Self("/machines/whatever/1") });
machinePolicies.Items.Add(new MachinePolicyResource {Id = "MachinePolicies-2", Name = "Machine Policy 2"});
operation.TentacleThumbprint = "ABCDEF";
operation.TentaclePort = 10930;
operation.MachineName = "Mymachine";
operation.TentacleHostname = "Mymachine.test.com";
operation.CommunicationStyle = CommunicationStyle.TentaclePassive;
operation.EnvironmentNames = new[] { "Production" };
operation.AllowOverwrite = true;
operation.MachinePolicy = "Machine Policy 2";

await operation.ExecuteAsync(serverEndpoint).ConfigureAwait(false);

await client.Received().Update("/machines/whatever/1", Arg.Is<MachineResource>(m =>
m.Id == "machines/84"
&& m.Name == "Mymachine"
&& m.EnvironmentIds.First() == "environments-2"
&& m.MachinePolicyId == "MachinePolicies-2"),
Arg.Any<object>(), Arg.Any<CancellationToken>()).ConfigureAwait(false);
}
}
}
6 changes: 6 additions & 0 deletions source/Octopus.Server.Client/Operations/RegisterKubernetesClusterOperation.cs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Threading.Tasks;
using Octopus.Client.Model;
using Octopus.Client.Model.Endpoints;

Expand All @@ -13,6 +14,11 @@ public RegisterKubernetesClusterOperation(IOctopusClientFactory clientFactory) :
{
}

protected override void PrepareMachineForReRegistration(MachineResource machine, string proxyId)
{
machine.Endpoint = GenerateEndpoint(proxyId);
}

protected override EndpointResource GenerateEndpoint(string proxyId)
{
return CommunicationStyle switch
Expand Down
75 changes: 51 additions & 24 deletions source/Octopus.Server.Client/Operations/RegisterMachineOperation.cs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Threading.Tasks;
using Octopus.Client.Exceptions;
using Octopus.Client.Model;
using Octopus.Client.Model.Endpoints;

namespace Octopus.Client.Operations
{
Expand Down Expand Up @@ -65,17 +63,32 @@ public RegisterMachineOperation(IOctopusClientFactory clientFactory) : base(clie
/// </exception>
public override void Execute(IOctopusSpaceRepository repository)
{
var selectedEnvironments = GetEnvironments(repository);
var machinePolicy = GetMachinePolicy(repository);
var machine = GetMachine(repository);
var tenants = GetTenants(repository);
ValidateTenantTags(repository);
var proxy = GetProxy(repository);

ApplyBaseChanges(machine, machinePolicy, proxy);
ApplyDeploymentTargetChanges(machine, selectedEnvironments, tenants);
if (!IsExistingMachine(machine) || AllowOverwrite)
{
var machinePolicy = GetMachinePolicy(repository);
ValidateTenantTags(repository);
ApplyBaseChanges(machine, machinePolicy, proxy);
ApplyDeploymentTargetChanges(machine, GetEnvironments(repository), GetTenants(repository));
}
else
{
PrepareMachineForReRegistration(machine, proxy?.Id);
}

ModifyOrCreateMachine(repository, machine);
}

static bool IsExistingMachine(MachineResource machine)
{
return machine.Id != null;
}

if (machine.Id != null)
static void ModifyOrCreateMachine(IOctopusSpaceRepository repository, MachineResource machine)
{
if (IsExistingMachine(machine))
repository.Machines.Modify(machine);
else
repository.Machines.Create(machine);
Expand Down Expand Up @@ -130,12 +143,11 @@ MachineResource GetMachine(IOctopusSpaceRepository repository)
try
{
existing = repository.Machines.FindByName(MachineName);
if (!AllowOverwrite && existing?.Id != null)
throw new InvalidRegistrationArgumentsException($"A machine named '{MachineName}' already exists on the Octopus Server in the target space. Use the 'force' parameter if you intended to update the existing machine.");
}
catch (OctopusDeserializationException) // eat it, probably caused by resource incompatability between versions
catch (OctopusDeserializationException) // eat it, probably caused by resource incompatibility between versions
{
}

return existing ?? new MachineResource();
}

Expand All @@ -147,18 +159,35 @@ MachineResource GetMachine(IOctopusSpaceRepository repository)
/// </exception>
public override async Task ExecuteAsync(IOctopusSpaceAsyncRepository repository)
{
var selectedEnvironments = GetEnvironments(repository).ConfigureAwait(false);
var machinePolicy = GetMachinePolicy(repository).ConfigureAwait(false);
var machineTask = GetMachine(repository).ConfigureAwait(false);
var tenants = GetTenants(repository).ConfigureAwait(false);
await ValidateTenantTags(repository).ConfigureAwait(false);
var proxy = GetProxy(repository).ConfigureAwait(false);
var machine = await GetMachine(repository).ConfigureAwait(false);
var proxy = await GetProxy(repository).ConfigureAwait(false);

if (!IsExistingMachine(machine) || AllowOverwrite)
{
var machinePolicy = GetMachinePolicy(repository).ConfigureAwait(false);
await ValidateTenantTags(repository).ConfigureAwait(false);
ApplyBaseChanges(machine, await machinePolicy, proxy);
var selectedEnvironments = await GetEnvironments(repository).ConfigureAwait(false);
var tenants = await GetTenants(repository).ConfigureAwait(false);
ApplyDeploymentTargetChanges(machine, selectedEnvironments, tenants);
}
else
{
PrepareMachineForReRegistration(machine, proxy?.Id);
}

await ModifyOrCreateMachine(repository, machine).ConfigureAwait(false);
}

var machine = await machineTask;
ApplyBaseChanges(machine, await machinePolicy, await proxy);
ApplyDeploymentTargetChanges(machine, await selectedEnvironments, await tenants);
protected virtual void PrepareMachineForReRegistration(MachineResource machineResource, string proxyId)
{
throw new InvalidRegistrationArgumentsException(
$"A machine named '{MachineName}' already exists in the environment. Use the 'force' parameter if you intended to update the existing machine.");
}

if (machine.Id != null)
static async Task ModifyOrCreateMachine(IOctopusSpaceAsyncRepository repository, MachineResource machine)
{
if (IsExistingMachine(machine))
await repository.Machines.Modify(machine).ConfigureAwait(false);
else
await repository.Machines.Create(machine).ConfigureAwait(false);
Expand Down Expand Up @@ -226,8 +255,6 @@ async Task<MachineResource> GetMachine(IOctopusSpaceAsyncRepository repository)
try
{
existing = await repository.Machines.FindByName(MachineName).ConfigureAwait(false);
if (!AllowOverwrite && existing?.Id != null)
throw new InvalidRegistrationArgumentsException($"A machine named '{MachineName}' already exists in the environment. Use the 'force' parameter if you intended to update the existing machine.");
}
catch (OctopusDeserializationException) // eat it, probably caused by resource incompatability between versions
{
Expand Down

0 comments on commit 22026c2

Please sign in to comment.