From fedc6e13ef01796e75f6594a66ba35d560ca489c Mon Sep 17 00:00:00 2001 From: Jason Zhang Date: Thu, 30 May 2024 13:56:04 -0400 Subject: [PATCH] Added unit tests and e2e for InstallPlan Signed-off-by: Jason Zhang --- controllers/operatorpolicy_status.go | 5 +- controllers/operatorpolicy_status_test.go | 170 ++++++++++------------ test/e2e/case38_install_operator_test.go | 63 ++++++-- 3 files changed, 126 insertions(+), 112 deletions(-) diff --git a/controllers/operatorpolicy_status.go b/controllers/operatorpolicy_status.go index ebfea84b..06028e54 100644 --- a/controllers/operatorpolicy_status.go +++ b/controllers/operatorpolicy_status.go @@ -1253,7 +1253,10 @@ func catalogSourceObj( } if isMissing { - compliance = string(policyv1.NonCompliant) + if complianceConfig != "Compliant" { + compliance = string(policyv1.NonCompliant) + } + reason = reasonWantFoundDNE } diff --git a/controllers/operatorpolicy_status_test.go b/controllers/operatorpolicy_status_test.go index 11b71d92..f8791785 100644 --- a/controllers/operatorpolicy_status_test.go +++ b/controllers/operatorpolicy_status_test.go @@ -7,103 +7,11 @@ import ( "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" policyv1beta1 "open-cluster-management.io/config-policy-controller/api/v1beta1" ) -func TestCalculateComplianceConditionComplianceConfig(t *testing.T) { - t.Parallel() - - testPolicy := &policyv1beta1.OperatorPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-policy", - Namespace: "default", - }, - Spec: policyv1beta1.OperatorPolicySpec{ - Severity: "low", - RemediationAction: "enforce", - ComplianceType: "musthave", - Subscription: runtime.RawExtension{ - Raw: []byte(`{ - "source": "my-catalog", - "sourceNamespace": "my-ns", - "name": "my-operator", - "channel": "stable", - "startingCSV": "my-operator-v1" - }`), - }, - ComplianceConfig: policyv1beta1.ComplianceConfig{ - CatalogSourceUnhealthy: "NonCompliant", - DeploymentsUnavailable: "NonCompliant", - UpgradesAvailable: "NonCompliant", - }, - }, - Status: policyv1beta1.OperatorPolicyStatus{ - Conditions: []metav1.Condition{ - { - Type: "ValidPolicySpec", - Status: "True", - }, - { - Type: "OperatorGroupCompliant", - Status: "True", - }, - { - Type: "SubscriptionCompliant", - Status: "True", - }, - { - Type: "InstallPlanCompliant", - Status: "True", - }, - { - Type: "ClusterServiceVersionCompliant", - Status: "True", - }, - { - Type: "CustomResourceDefinitionCompliant", - Status: "True", - }, - { - Type: "DeploymentCompliant", - Status: "True", - }, - { - Type: "CatalogSourcesUnhealthy", - Status: "False", - }, - }, - }, - } - - // upgradesAvailable - testPolicy.Status.Conditions[3].Status = "False" - complianceCond := calculateComplianceCondition(testPolicy) - assert.Equal(t, "NonCompliant", complianceCond.Reason) - - testPolicy.Spec.ComplianceConfig.UpgradesAvailable = "Compliant" - complianceCond = calculateComplianceCondition(testPolicy) - assert.Equal(t, "Compliant", complianceCond.Reason) - - // CatalogSourcesUnhealthy - testPolicy.Status.Conditions[7].Status = "True" - complianceCond = calculateComplianceCondition(testPolicy) - assert.Equal(t, "NonCompliant", complianceCond.Reason) - - testPolicy.Spec.ComplianceConfig.CatalogSourceUnhealthy = "Compliant" - complianceCond = calculateComplianceCondition(testPolicy) - assert.Equal(t, "Compliant", complianceCond.Reason) - - // DeploymentsUnavailable - testPolicy.Status.Conditions[6].Status = "False" - complianceCond = calculateComplianceCondition(testPolicy) - assert.Equal(t, "NonCompliant", complianceCond.Reason) - - testPolicy.Spec.ComplianceConfig.DeploymentsUnavailable = "Compliant" - complianceCond = calculateComplianceCondition(testPolicy) - assert.Equal(t, "Compliant", complianceCond.Reason) -} +var overrideMsg = " - ignore, compliance overridden by complianceConfig" func TestExistingInstallPlanObj(t *testing.T) { // Empty InstallPlan @@ -127,6 +35,47 @@ func TestExistingInstallPlanObj(t *testing.T) { assert.Equal(t, "NonCompliant", res.Compliant) } +func TestBuildDeploymentCond(t *testing.T) { + complianceConfig := policyv1beta1.Compliant + depsExist := true // if any deployments + unavailableDeps := make([]appsv1.Deployment, 0, 1) // deployments are there, but no available + + // Test Compliant complianceConfig with available deployments + cond := buildDeploymentCond(complianceConfig, depsExist, unavailableDeps) + assert.Equal(t, metav1.ConditionTrue, cond.Status) + assert.NotContains(t, cond.Message, overrideMsg) + + // Test NonCompliant complianceConfig with available deployments + complianceConfig = policyv1beta1.NonCompliant + cond = buildDeploymentCond(complianceConfig, depsExist, unavailableDeps) + assert.Equal(t, metav1.ConditionTrue, cond.Status) + assert.Equal(t, "all operator Deployments have their minimum availability", cond.Message) + + // Test Compliant complianceConfig with unavailable deployments + testDeployment := appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "TestDeployment", + }, + Spec: appsv1.DeploymentSpec{}, + Status: appsv1.DeploymentStatus{ + UnavailableReplicas: 1, + }, + } + complianceConfig = policyv1beta1.Compliant + + unavailableDeps = append(unavailableDeps, testDeployment) + cond = buildDeploymentCond(complianceConfig, depsExist, unavailableDeps) + assert.Equal(t, metav1.ConditionTrue, cond.Status) + assert.Contains(t, cond.Message, overrideMsg) + + // Test NonCompliance complianceConfig with unavailable deployments + complianceConfig = policyv1beta1.NonCompliant + cond = buildDeploymentCond(complianceConfig, depsExist, unavailableDeps) + assert.Equal(t, metav1.ConditionFalse, cond.Status) + assert.Equal(t, "the deployments TestDeployment do not have their minimum availability", cond.Message) +} + func TestExistingDeploymentObj(t *testing.T) { testDeployment := &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{}, @@ -165,11 +114,42 @@ func TestExistingDeploymentObj(t *testing.T) { assert.Equal(t, "Deployment Available", res.Reason) } +func TestCatalogSourceFindCond(t *testing.T) { + complianceConfig := policyv1beta1.Compliant + isUnhealthy := false + isMissing := false + name := "TestCatalog" + + // Test Compliant complianceConfig with healthy CatalogSource + cond := catalogSourceFindCond(complianceConfig, isUnhealthy, isMissing, name) + assert.Equal(t, metav1.ConditionFalse, cond.Status) + assert.NotContains(t, cond.Message, overrideMsg) + + // Test Compliant complianceConfig with unhealthy CatalogSource + isUnhealthy = true + cond = catalogSourceFindCond(complianceConfig, isUnhealthy, isMissing, name) + assert.Equal(t, metav1.ConditionFalse, cond.Status) + assert.Contains(t, cond.Message, overrideMsg) + + // Test NonCompliant complianceConfig with healthy CatalogSource + isUnhealthy = false + complianceConfig = policyv1beta1.NonCompliant + cond = catalogSourceFindCond(complianceConfig, isUnhealthy, isMissing, name) + assert.Equal(t, metav1.ConditionFalse, cond.Status) + assert.Equal(t, "CatalogSource was found", cond.Message) + + // Test NonCompliant complianceConfig with unhealthy CatalogSource + isUnhealthy = true + cond = catalogSourceFindCond(complianceConfig, isUnhealthy, isMissing, name) + assert.Equal(t, metav1.ConditionTrue, cond.Status) + assert.Equal(t, "CatalogSource was found but is unhealthy", cond.Message) +} + func TestCatalogSourceObj(t *testing.T) { catalogName := "TestCatalog" catalogNS := "TestNamespace" - // Test Compliant complianceConfig with no healthy CatalogSource + // Test Compliant complianceConfig with healthy CatalogSource isUnhealthy := false isMissing := false complianceConfig := policyv1beta1.ComplianceConfigAction("Compliant") @@ -185,7 +165,7 @@ func TestCatalogSourceObj(t *testing.T) { assert.Equal(t, "Compliant", res.Compliant) assert.Equal(t, "Resource found as expected but is unhealthy", res.Reason) - // Test NonCompliant complianceConfig with no healthy CatalogSource + // Test NonCompliant complianceConfig with healthy CatalogSource isUnhealthy = false isMissing = false complianceConfig = "NonCompliant" diff --git a/test/e2e/case38_install_operator_test.go b/test/e2e/case38_install_operator_test.go index cb30c638..4e535b4c 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -1107,7 +1107,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu By("Checking the conditions and relatedObj in the policy") check( OpPlcName, - true, + false, []policyv1.RelatedObject{{ Object: policyv1.ObjectResource{ Kind: "CatalogSource", @@ -1117,14 +1117,15 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu Namespace: opPolTestNS, }, }, - Compliant: "NonCompliant", + Compliant: "Compliant", Reason: "Resource not found but should exist", }}, metav1.Condition{ - Type: "CatalogSourcesUnhealthy", - Status: metav1.ConditionTrue, - Reason: "CatalogSourcesNotFound", - Message: "CatalogSource 'fakeName' was not found", + Type: "CatalogSourcesUnhealthy", + Status: metav1.ConditionFalse, + Reason: "CatalogSourcesNotFound", + Message: "CatalogSource 'fakeName' was not found" + + " - ignore, compliance overridden by complianceConfig", }, "CatalogSource 'fakeName' was not found", ) @@ -1141,7 +1142,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu By("Checking the conditions and relatedObj in the policy") check( OpPlcName, - true, + false, []policyv1.RelatedObject{{ Object: policyv1.ObjectResource{ Kind: "CatalogSource", @@ -1151,14 +1152,15 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu Namespace: opPolTestNS, }, }, - Compliant: "NonCompliant", + Compliant: "Compliant", Reason: "Resource found as expected but is unhealthy", }}, metav1.Condition{ - Type: "CatalogSourcesUnhealthy", - Status: metav1.ConditionTrue, - Reason: "CatalogSourcesFoundUnhealthy", - Message: "CatalogSource was found but is unhealthy", + Type: "CatalogSourcesUnhealthy", + Status: metav1.ConditionFalse, + Reason: "CatalogSourcesFoundUnhealthy", + Message: "CatalogSource was found but is unhealthy" + + " - ignore, compliance overridden by complianceConfig", }, "CatalogSource was found but is unhealthy", ) @@ -1166,8 +1168,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu It("Should become Compliant when ComplianceConfig is modified", func() { By("Patching the policy ComplianceConfig to Compliant") utils.Kubectl("patch", "operatorpolicy", OpPlcName, "-n", opPolTestNS, "--type=json", "-p", - `[{"op": "replace", "path": "/spec/complianceConfig/catalogSourceUnhealthy", - "value": "Compliant"}]`) + `[{"op": "replace", "path": "/spec/complianceConfig/catalogSourceUnhealthy", "value": "Compliant"}]`) By("Checking the conditions and relatedObj in the policy") check( @@ -1187,7 +1188,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu }}, metav1.Condition{ Type: "CatalogSourcesUnhealthy", - Status: metav1.ConditionTrue, + Status: metav1.ConditionFalse, Reason: "CatalogSourcesFoundUnhealthy", Message: "CatalogSource was found but is unhealthy", }, @@ -1303,6 +1304,36 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu return len(ipList.Items) }, olmWaitTimeout, 5, ctx).Should(Equal(1)) + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "InstallPlan", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Namespace: opPolTestNS, + }, + }, + Compliant: "Compliant", + Reason: "The InstallPlan is RequiresApproval", + }}, + metav1.Condition{ + Type: "InstallPlanCompliant", + Status: metav1.ConditionTrue, + Reason: "InstallPlanRequiresApproval", + Message: "an InstallPlan to update to [strimzi-cluster-operator.v0.36.0] is available for approval" + + " - ignore, compliance overridden by complianceConfig", + }, + "an InstallPlan to update .* is available for approval", + ) + }) + It("Should become NonCompliant when ComplianceConfig is modified to NonCompliant", func(ctx SpecContext) { + By("Patching the policy ComplianceConfig to Compliant") + utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/complianceConfig/upgradesAvailable", "value": "NonCompliant"}]`) + + By("Checking the conditions and relatedObj in the policy") check( opPolName, true, @@ -3018,7 +3049,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu Expect(remBehavior).To(HaveKeyWithValue("customResourceDefinitions", "Keep")) }) }) - FDescribe("Testing defaulted values of ComplianceConfig in an OperatorPolicy", func() { + Describe("Testing defaulted values of ComplianceConfig in an OperatorPolicy", func() { const ( opPolYAML = "../resources/case38_operator_install/operator-policy-no-group.yaml" opPolName = "oppol-no-group"