review fixes

canonical · Sep 30, 2024 · c813a7c · c813a7c
1 parent d5d9439
commit c813a7c
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 39 deletions.
diff --git 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
@@ -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
@@ -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,15 +142,15 @@ 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
 }
 

diff --git 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))
 	})
 }