From 72d4456b3fe6daccf25a1e230d6627b61c440d11 Mon Sep 17 00:00:00 2001 From: Nils Hanke Date: Tue, 6 Sep 2022 10:28:00 +0200 Subject: [PATCH] GCP: Only create debugd loadbalancer when debugCluster is set --- cli/internal/cloudcmd/clients.go | 2 +- cli/internal/cloudcmd/clients_test.go | 4 +- cli/internal/cloudcmd/create.go | 2 +- cli/internal/gcp/client/loadbalancer.go | 19 ++++---- cli/internal/gcp/client/loadbalancer_test.go | 49 ++++++++++++++++---- 5 files changed, 54 insertions(+), 22 deletions(-) diff --git a/cli/internal/cloudcmd/clients.go b/cli/internal/cloudcmd/clients.go index 9150437e6..e591deaf2 100644 --- a/cli/internal/cloudcmd/clients.go +++ b/cli/internal/cloudcmd/clients.go @@ -20,7 +20,7 @@ type gcpclient interface { CreateVPCs(ctx context.Context) error CreateFirewall(ctx context.Context, input gcpcl.FirewallInput) error CreateInstances(ctx context.Context, input gcpcl.CreateInstancesInput) error - CreateLoadBalancers(ctx context.Context) error + CreateLoadBalancers(ctx context.Context, isDebugCluster bool) error TerminateFirewall(ctx context.Context) error TerminateVPCs(context.Context) error TerminateLoadBalancers(context.Context) error diff --git a/cli/internal/cloudcmd/clients_test.go b/cli/internal/cloudcmd/clients_test.go index 81cb26b64..7a1ad48e3 100644 --- a/cli/internal/cloudcmd/clients_test.go +++ b/cli/internal/cloudcmd/clients_test.go @@ -284,7 +284,7 @@ func (c *fakeGcpClient) CreateInstances(ctx context.Context, input gcpcl.CreateI return nil } -func (c *fakeGcpClient) CreateLoadBalancers(ctx context.Context) error { +func (c *fakeGcpClient) CreateLoadBalancers(ctx context.Context, isDebugCluster bool) error { c.loadbalancers = []string{"kube-lb", "boot-lb", "verify-lb"} return nil } @@ -361,7 +361,7 @@ func (c *stubGcpClient) CreateInstances(ctx context.Context, input gcpcl.CreateI return c.createInstancesErr } -func (c *stubGcpClient) CreateLoadBalancers(ctx context.Context) error { +func (c *stubGcpClient) CreateLoadBalancers(ctx context.Context, isDebugClient bool) error { return c.createLoadBalancerErr } diff --git a/cli/internal/cloudcmd/create.go b/cli/internal/cloudcmd/create.go index 512823d76..550c1abc0 100644 --- a/cli/internal/cloudcmd/create.go +++ b/cli/internal/cloudcmd/create.go @@ -150,7 +150,7 @@ func (c *Creator) createGCP(ctx context.Context, cl gcpclient, config *config.Co return state.ConstellationState{}, err } - if err := cl.CreateLoadBalancers(ctx); err != nil { + if err := cl.CreateLoadBalancers(ctx, config.IsDebugCluster()); err != nil { return state.ConstellationState{}, err } diff --git a/cli/internal/gcp/client/loadbalancer.go b/cli/internal/gcp/client/loadbalancer.go index 620618b1f..907b6f401 100644 --- a/cli/internal/gcp/client/loadbalancer.go +++ b/cli/internal/gcp/client/loadbalancer.go @@ -35,7 +35,7 @@ type loadBalancer struct { } // CreateLoadBalancers creates all necessary load balancers. -func (c *Client) CreateLoadBalancers(ctx context.Context) error { +func (c *Client) CreateLoadBalancers(ctx context.Context, isDebugCluster bool) error { if err := c.createIPAddr(ctx); err != nil { return fmt.Errorf("creating load balancer IP address: %w", err) } @@ -69,13 +69,16 @@ func (c *Client) CreateLoadBalancers(ctx context.Context) error { healthCheck: computepb.HealthCheck_TCP, }) - c.loadbalancers = append(c.loadbalancers, &loadBalancer{ - name: c.buildResourceName("debugd"), - ip: c.loadbalancerIPname, - frontendPort: constants.DebugdPort, - backendPortName: "debugd", - healthCheck: computepb.HealthCheck_TCP, - }) + // Only create when the debug cluster flag is set in the Constellation config + if isDebugCluster { + c.loadbalancers = append(c.loadbalancers, &loadBalancer{ + name: c.buildResourceName("debugd"), + ip: c.loadbalancerIPname, + frontendPort: constants.DebugdPort, + backendPortName: "debugd", + healthCheck: computepb.HealthCheck_TCP, + }) + } // Load balancer creation. diff --git a/cli/internal/gcp/client/loadbalancer_test.go b/cli/internal/gcp/client/loadbalancer_test.go index 706f7b700..bcf2ef9e7 100644 --- a/cli/internal/gcp/client/loadbalancer_test.go +++ b/cli/internal/gcp/client/loadbalancer_test.go @@ -9,6 +9,7 @@ package client import ( "context" "errors" + "fmt" "net/http" "testing" @@ -23,12 +24,13 @@ func TestCreateLoadBalancers(t *testing.T) { forwardingRule := &compute.ForwardingRule{LabelFingerprint: proto.String("fingerprint")} testCases := map[string]struct { - addrAPI addressesAPI - healthAPI healthChecksAPI - backendAPI backendServicesAPI - forwardAPI forwardingRulesAPI - opRegAPI operationRegionAPI - wantErr bool + addrAPI addressesAPI + healthAPI healthChecksAPI + backendAPI backendServicesAPI + forwardAPI forwardingRulesAPI + opRegAPI operationRegionAPI + isDebugCluster bool + wantErr bool }{ "successful create": { addrAPI: &stubAddressesAPI{getAddr: proto.String("192.0.2.1")}, @@ -37,6 +39,14 @@ func TestCreateLoadBalancers(t *testing.T) { forwardAPI: &stubForwardingRulesAPI{forwardingRule: forwardingRule}, opRegAPI: stubOperationRegionAPI{}, }, + "successful create (debug cluster)": { + addrAPI: &stubAddressesAPI{getAddr: proto.String("192.0.2.1")}, + healthAPI: &stubHealthChecksAPI{}, + backendAPI: &stubBackendServicesAPI{}, + forwardAPI: &stubForwardingRulesAPI{forwardingRule: forwardingRule}, + opRegAPI: stubOperationRegionAPI{}, + isDebugCluster: true, + }, "createIPAddr fails": { addrAPI: &stubAddressesAPI{insertErr: someErr}, healthAPI: &stubHealthChecksAPI{}, @@ -72,14 +82,33 @@ func TestCreateLoadBalancers(t *testing.T) { operationRegionAPI: tc.opRegAPI, } - err := client.CreateLoadBalancers(ctx) + err := client.CreateLoadBalancers(ctx, tc.isDebugCluster) + // In case we expect an error, check for the error and continue otherwise. if tc.wantErr { assert.Error(err) - } else { - assert.NoError(err) - assert.NotEmpty(client.loadbalancerIPname) + return + } + + // If we don't expect an error, check if the resources have been successfully created. + assert.NoError(err) + assert.NotEmpty(client.loadbalancerIPname) + + var foundDebugdLB bool + for _, lb := range client.loadbalancers { + // Expect load balancer name to have the format of "name-serviceName-uid" which is what buildResourceName does currently. + if lb.name == fmt.Sprintf("%s-debugd-%s", client.name, client.uid) { + foundDebugdLB = true + break + } + } + + if tc.isDebugCluster { assert.Equal(4, len(client.loadbalancers)) + assert.True(foundDebugdLB, "debugd loadbalancer not found in debug-mode") + } else { + assert.Equal(3, len(client.loadbalancers)) + assert.False(foundDebugdLB, "debugd loadbalancer found in non-debug mode") } }) }