From c813a7ccbb8849bc71ff939fa7719d32c75917dc Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Mon, 30 Sep 2024 18:00:41 +0200 Subject: [PATCH] review fixes --- src/k8s/pkg/k8sd/features/contour/gateway.go | 20 +++++------ .../pkg/k8sd/features/contour/gateway_test.go | 20 +++++++---- src/k8s/pkg/k8sd/features/contour/ingress.go | 24 ++++++------- .../pkg/k8sd/features/contour/ingress_test.go | 35 +++++++++++++------ 4 files changed, 60 insertions(+), 39 deletions(-) diff --git a/src/k8s/pkg/k8sd/features/contour/gateway.go b/src/k8s/pkg/k8sd/features/contour/gateway.go index c1e092d32..320140d3b 100644 --- a/src/k8s/pkg/k8sd/features/contour/gateway.go +++ b/src/k8s/pkg/k8sd/features/contour/gateway.go @@ -11,10 +11,10 @@ import ( ) const ( - enabledMsg = "enabled" - disabledMsg = "disabled" - gatewayDeployFailedMsgTmpl = "Failed to deploy Contour Gateway, the error was: %v" - gatewayDeleteFailedMsgTmpl = "Failed to delete Contour Gateway, the error was: %v" + EnabledMsg = "enabled" + DisabledMsg = "disabled" + GatewayDeployFailedMsgTmpl = "Failed to deploy Contour Gateway, the error was: %v" + GatewayDeleteFailedMsgTmpl = "Failed to delete Contour Gateway, the error was: %v" ) // ApplyGateway will install a helm chart for contour-gateway-provisioner on the cluster when gateway.Enabled is true. @@ -33,13 +33,13 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne return types.FeatureStatus{ Enabled: false, Version: ContourGatewayProvisionerContourImageTag, - Message: fmt.Sprintf(gatewayDeleteFailedMsgTmpl, err), + Message: fmt.Sprintf(GatewayDeleteFailedMsgTmpl, err), }, err } return types.FeatureStatus{ Enabled: false, Version: ContourGatewayProvisionerContourImageTag, - Message: disabledMsg, + Message: DisabledMsg, }, nil } @@ -49,7 +49,7 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne return types.FeatureStatus{ Enabled: false, Version: ContourGatewayProvisionerContourImageTag, - Message: fmt.Sprintf(gatewayDeployFailedMsgTmpl, err), + Message: fmt.Sprintf(GatewayDeployFailedMsgTmpl, err), }, err } @@ -58,7 +58,7 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne return types.FeatureStatus{ Enabled: false, Version: ContourGatewayProvisionerContourImageTag, - Message: fmt.Sprintf(gatewayDeployFailedMsgTmpl, err), + Message: fmt.Sprintf(GatewayDeployFailedMsgTmpl, err), }, err } @@ -82,14 +82,14 @@ func ApplyGateway(ctx context.Context, snap snap.Snap, gateway types.Gateway, ne return types.FeatureStatus{ Enabled: false, Version: ContourGatewayProvisionerContourImageTag, - Message: fmt.Sprintf(gatewayDeployFailedMsgTmpl, err), + Message: fmt.Sprintf(GatewayDeployFailedMsgTmpl, err), }, err } return types.FeatureStatus{ Enabled: true, Version: ContourGatewayProvisionerContourImageTag, - Message: enabledMsg, + Message: EnabledMsg, }, nil } diff --git a/src/k8s/pkg/k8sd/features/contour/gateway_test.go b/src/k8s/pkg/k8sd/features/contour/gateway_test.go index 971f97f09..24bfd6652 100644 --- a/src/k8s/pkg/k8sd/features/contour/gateway_test.go +++ b/src/k8s/pkg/k8sd/features/contour/gateway_test.go @@ -3,6 +3,7 @@ package contour_test import ( "context" "errors" + "fmt" "testing" "time" @@ -43,7 +44,9 @@ func TestGatewayDisabled(t *testing.T) { g.Expect(err.Error()).To(ContainSubstring(applyErr.Error())) g.Expect(status.Enabled).To(BeFalse()) g.Expect(status.Version).To(Equal(contour.ContourGatewayProvisionerContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("Failed to delete Contour Gateway")) + g.Expect(status.Message).To(Equal(fmt.Sprintf(contour.GatewayDeleteFailedMsgTmpl, + fmt.Errorf("failed to uninstall the contour gateway chart: %w", applyErr), + ))) g.Expect(helmM.ApplyCalledWith).To(HaveLen(1)) }) @@ -67,7 +70,7 @@ func TestGatewayDisabled(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(status.Enabled).To(BeFalse()) g.Expect(status.Version).To(Equal(contour.ContourGatewayProvisionerContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("disabled")) + g.Expect(status.Message).To(Equal(contour.DisabledMsg)) g.Expect(helmM.ApplyCalledWith).To(HaveLen(1)) }) @@ -94,10 +97,12 @@ func TestGatewayEnabled(t *testing.T) { status, err := contour.ApplyGateway(context.Background(), snapM, gateway, network, nil) g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring(applyErr.Error())) + g.Expect(err).To(MatchError(applyErr)) g.Expect(status.Enabled).To(BeFalse()) g.Expect(status.Version).To(Equal(contour.ContourGatewayProvisionerContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("Failed to deploy Contour Gateway")) + g.Expect(status.Message).To(Equal(fmt.Sprintf(contour.GatewayDeployFailedMsgTmpl, + fmt.Errorf("failed to apply common contour CRDS: %w", fmt.Errorf("failed to install common CRDS: %w", applyErr)), + ))) g.Expect(helmM.ApplyCalledWith).To(HaveLen(1)) }) @@ -144,7 +149,7 @@ func TestGatewayEnabled(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(status.Enabled).To(BeTrue()) g.Expect(status.Version).To(Equal(contour.ContourGatewayProvisionerContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("enabled")) + g.Expect(status.Message).To(Equal(contour.EnabledMsg)) g.Expect(helmM.ApplyCalledWith).To(HaveLen(2)) values := helmM.ApplyCalledWith[1].Values @@ -198,7 +203,10 @@ func TestGatewayEnabled(t *testing.T) { g.Expect(err.Error()).To(ContainSubstring("failed to wait for required contour common CRDs")) g.Expect(status.Enabled).To(BeFalse()) g.Expect(status.Version).To(Equal(contour.ContourGatewayProvisionerContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("Failed to deploy Contour Gateway")) + g.Expect(status.Message).To(Equal(fmt.Sprintf(contour.GatewayDeployFailedMsgTmpl, + fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", + errors.New("context deadline exceeded")), + ))) g.Expect(helmM.ApplyCalledWith).To(HaveLen(1)) }) } diff --git a/src/k8s/pkg/k8sd/features/contour/ingress.go b/src/k8s/pkg/k8sd/features/contour/ingress.go index a56a43c40..cf2f45ebe 100644 --- a/src/k8s/pkg/k8sd/features/contour/ingress.go +++ b/src/k8s/pkg/k8sd/features/contour/ingress.go @@ -11,8 +11,8 @@ import ( ) const ( - ingressDeleteFailedMsgTmpl = "Failed to delete Contour Ingress, the error was: %v" - ingressDeployFailedMsgTmpl = "Failed to deploy Contour Ingress, the error was: %v" + IngressDeleteFailedMsgTmpl = "Failed to delete Contour Ingress, the error was: %v" + IngressDeployFailedMsgTmpl = "Failed to deploy Contour Ingress, the error was: %v" ) // ApplyIngress will install the contour helm chart when ingress.Enabled is true. @@ -34,13 +34,13 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ return types.FeatureStatus{ Enabled: false, Version: ContourIngressContourImageTag, - Message: fmt.Sprintf(ingressDeleteFailedMsgTmpl, err), + Message: fmt.Sprintf(IngressDeleteFailedMsgTmpl, err), }, err } return types.FeatureStatus{ Enabled: false, Version: ContourIngressContourImageTag, - Message: disabledMsg, + Message: DisabledMsg, }, nil } @@ -50,7 +50,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ return types.FeatureStatus{ Enabled: false, Version: ContourIngressContourImageTag, - Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + Message: fmt.Sprintf(IngressDeployFailedMsgTmpl, err), }, err } @@ -59,7 +59,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ return types.FeatureStatus{ Enabled: false, Version: ContourIngressContourImageTag, - Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + Message: fmt.Sprintf(IngressDeployFailedMsgTmpl, err), }, err } @@ -100,7 +100,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ return types.FeatureStatus{ Enabled: false, Version: ContourIngressContourImageTag, - Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + Message: fmt.Sprintf(IngressDeployFailedMsgTmpl, err), }, err } @@ -110,7 +110,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ return types.FeatureStatus{ Enabled: false, Version: ContourIngressContourImageTag, - Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + Message: fmt.Sprintf(IngressDeployFailedMsgTmpl, err), }, err } } @@ -127,13 +127,13 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ return types.FeatureStatus{ Enabled: false, Version: ContourIngressContourImageTag, - Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + Message: fmt.Sprintf(IngressDeployFailedMsgTmpl, err), }, err } return types.FeatureStatus{ Enabled: true, Version: ContourIngressContourImageTag, - Message: enabledMsg, + Message: EnabledMsg, }, nil } @@ -142,7 +142,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ return types.FeatureStatus{ Enabled: false, Version: ContourIngressContourImageTag, - Message: fmt.Sprintf(ingressDeployFailedMsgTmpl, err), + Message: fmt.Sprintf(IngressDeployFailedMsgTmpl, err), }, err } @@ -150,7 +150,7 @@ func ApplyIngress(ctx context.Context, snap snap.Snap, ingress types.Ingress, _ return types.FeatureStatus{ Enabled: true, Version: ContourIngressContourImageTag, - Message: enabledMsg, + Message: EnabledMsg, }, nil } diff --git a/src/k8s/pkg/k8sd/features/contour/ingress_test.go b/src/k8s/pkg/k8sd/features/contour/ingress_test.go index 8346e0499..60f69d0c8 100644 --- a/src/k8s/pkg/k8sd/features/contour/ingress_test.go +++ b/src/k8s/pkg/k8sd/features/contour/ingress_test.go @@ -3,6 +3,7 @@ package contour_test import ( "context" "errors" + "fmt" "testing" "time" @@ -41,10 +42,12 @@ func TestIngressDisabled(t *testing.T) { status, err := contour.ApplyIngress(context.Background(), snapM, ingress, network, nil) g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring(applyErr.Error())) + g.Expect(err).To(MatchError(applyErr)) g.Expect(status.Enabled).To(BeFalse()) g.Expect(status.Version).To(Equal(contour.ContourIngressContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("failed to uninstall ingress")) + g.Expect(status.Message).To(Equal(fmt.Sprintf(contour.IngressDeleteFailedMsgTmpl, + fmt.Errorf("failed to uninstall ingress: %w", applyErr), + ))) g.Expect(helmM.ApplyCalledWith).To(HaveLen(1)) }) @@ -67,7 +70,7 @@ func TestIngressDisabled(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(status.Enabled).To(BeFalse()) g.Expect(status.Version).To(Equal(contour.ContourIngressContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("disabled")) + g.Expect(status.Message).To(Equal(contour.DisabledMsg)) g.Expect(helmM.ApplyCalledWith).To(HaveLen(1)) }) } @@ -93,10 +96,12 @@ func TestIngressEnabled(t *testing.T) { status, err := contour.ApplyIngress(context.Background(), snapM, ingress, network, nil) g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring(applyErr.Error())) + g.Expect(err).To(MatchError(applyErr)) g.Expect(status.Enabled).To(BeFalse()) g.Expect(status.Version).To(Equal(contour.ContourIngressContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("Failed to deploy Contour Ingress")) + g.Expect(status.Message).To(Equal(fmt.Sprintf(contour.IngressDeployFailedMsgTmpl, + fmt.Errorf("failed to apply common contour CRDS: %w", fmt.Errorf("failed to install common CRDS: %w", applyErr)), + ))) g.Expect(helmM.ApplyCalledWith).To(HaveLen(1)) }) @@ -151,7 +156,7 @@ func TestIngressEnabled(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(status.Enabled).To(BeTrue()) g.Expect(status.Version).To(Equal(contour.ContourIngressContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("enabled")) + g.Expect(status.Message).To(Equal(contour.EnabledMsg)) g.Expect(helmM.ApplyCalledWith).To(HaveLen(3)) validateIngressValues(g, helmM.ApplyCalledWith[1].Values, ingress) }) @@ -208,7 +213,7 @@ func TestIngressEnabled(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(status.Enabled).To(BeTrue()) g.Expect(status.Version).To(Equal(contour.ContourIngressContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("enabled")) + g.Expect(status.Message).To(Equal(contour.EnabledMsg)) g.Expect(helmM.ApplyCalledWith).To(HaveLen(3)) validateIngressValues(g, helmM.ApplyCalledWith[1].Values, ingress) }) @@ -267,7 +272,7 @@ func TestIngressEnabled(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(status.Enabled).To(BeTrue()) g.Expect(status.Version).To(Equal(contour.ContourIngressContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("enabled")) + g.Expect(status.Message).To(Equal(contour.EnabledMsg)) g.Expect(helmM.ApplyCalledWith).To(HaveLen(3)) validateIngressValues(g, helmM.ApplyCalledWith[1].Values, ingress) g.Expect(helmM.ApplyCalledWith[2].Values["defaultTLSSecret"]).To(Equal(defaultTLSSecret)) @@ -313,10 +318,12 @@ func TestIngressEnabled(t *testing.T) { status, err := contour.ApplyIngress(ctx, snapM, ingress, network, nil) g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring("failed to wait for required contour common CRDs")) g.Expect(status.Enabled).To(BeFalse()) g.Expect(status.Version).To(Equal(contour.ContourIngressContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("Failed to deploy Contour Ingress")) + g.Expect(status.Message).To(Equal(fmt.Sprintf(contour.IngressDeployFailedMsgTmpl, + fmt.Errorf("failed to wait for required contour common CRDs to be available: %w", + errors.New("context deadline exceeded")), + ))) g.Expect(helmM.ApplyCalledWith).To(HaveLen(1)) }) @@ -372,7 +379,13 @@ func TestIngressEnabled(t *testing.T) { g.Expect(err.Error()).To(ContainSubstring("failed to rollout restart contour to apply ingress")) g.Expect(status.Enabled).To(BeFalse()) g.Expect(status.Version).To(Equal(contour.ContourIngressContourImageTag)) - g.Expect(status.Message).To(ContainSubstring("Failed to deploy Contour Ingress")) + g.Expect(status.Message).To(Equal(fmt.Sprintf(contour.IngressDeployFailedMsgTmpl, + fmt.Errorf("failed to rollout restart contour to apply ingress: %v", + fmt.Errorf("failed to restart contour deployment after 3 attempts: %w", + fmt.Errorf("failed to restart contour deployment: %w", + fmt.Errorf("failed to get deployment ck-ingress-contour-contour in namespace projectcontour: %w", + errors.New("deployments.apps \"ck-ingress-contour-contour\" not found"))))), + ))) g.Expect(helmM.ApplyCalledWith).To(HaveLen(2)) }) }