Update v1alpha2 HNCConfig modes to camel case

Update v1alpha2 HNCConfig sync modes to camel case, e.g. 'propagate' to 'Propagate'. In addition, add e2e test case to make sure v1alpha2 HNCConfig reconciler is working as expected. Verify v1alpha1 'propagate' is not treated as unknown and converted to v1alpha2 'Ignore'. Verify object propagation is still working after conversion. Tested by 'make test-conversion'. When the old cached image was used, v1alpha1 'propagate' was converted to v1alpha2 'Ignore'. After the image was re-pulled, it was converted to v1alpha2 'Propagate' correctly.
kubernetes-retired · Aug 20, 2020 · 9a9e4a7 · 9a9e4a7
1 parent 8c65c19
commit 9a9e4a7
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 50 deletions.
diff --git a/incubator/hnc/api/v1alpha2/hnc_config.go b/incubator/hnc/api/v1alpha2/hnc_config.go
@@ -26,21 +26,21 @@ const (
 )
 
 // SynchronizationMode describes propagation mode of objects of the same kind.
-// The only three modes currently supported are "propagate", "ignore", and "remove".
+// The only three modes currently supported are "Propagate", "Ignore", and "Remove".
 // See detailed definition below. An unsupported mode will be treated as "ignore".
 type SynchronizationMode string
 
 const (
 	// Propagate objects from ancestors to descendants and deletes obsolete descendants.
-	Propagate SynchronizationMode = "propagate"
+	Propagate SynchronizationMode = "Propagate"
 
 	// Ignore the modification of this type. New or changed objects will not be propagated,
 	// and obsolete objects will not be deleted. The inheritedFrom label is not removed.
-	// Any unknown mode is treated as ignore.
-	Ignore SynchronizationMode = "ignore"
+	// Any unknown mode is treated as Ignore.
+	Ignore SynchronizationMode = "Ignore"
 
 	// Remove all existing propagated copies.
-	Remove SynchronizationMode = "remove"
+	Remove SynchronizationMode = "Remove"
 )
 
 // HNCConfigurationCondition codes. *All* codes must also be documented in the
@@ -57,9 +57,9 @@ type TypeSynchronizationSpec struct {
 	// Kind to be configured.
 	Kind string `json:"kind"`
 	// Synchronization mode of the kind. If the field is empty, it will be treated
-	// as "propagate".
+	// as "Propagate".
 	// +optional
-	// +kubebuilder:validation:Enum=propagate;ignore;remove
+	// +kubebuilder:validation:Enum=Propagate;Ignore;Remove
 	Mode SynchronizationMode `json:"mode,omitempty"`
 }
 

diff --git a/incubator/hnc/config/crd/bases/hnc.x-k8s.io_hncconfigurations.yaml b/incubator/hnc/config/crd/bases/hnc.x-k8s.io_hncconfigurations.yaml
@@ -42,11 +42,11 @@ spec:
                     description: Kind to be configured.
                     type: string
                   mode:
-                    description: Synchronization mode of the kind. If the field is empty, it will be treated as "propagate".
+                    description: Synchronization mode of the kind. If the field is empty, it will be treated as "Propagate".
                     enum:
-                    - propagate
-                    - ignore
-                    - remove
+                    - Propagate
+                    - Ignore
+                    - Remove
                     type: string
                 required:
                 - apiVersion

diff --git a/incubator/hnc/internal/validators/hncconfig_test.go b/incubator/hnc/internal/validators/hncconfig_test.go
@@ -56,10 +56,10 @@ func TestRBACTypes(t *testing.T) {
 		allow   bool
 	}{
 		{
-			name: "Correct RBAC config with propagate mode",
+			name: "Correct RBAC config with Propagate mode",
 			configs: []api.TypeSynchronizationSpec{
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "propagate"},
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "Propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "Propagate"},
 			},
 			allow: true,
 		},
@@ -74,44 +74,44 @@ func TestRBACTypes(t *testing.T) {
 		{
 			name: "Missing Role",
 			configs: []api.TypeSynchronizationSpec{
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "Propagate"},
 			},
 			allow: false,
 		}, {
 			name: "Missing RoleBinding",
 			configs: []api.TypeSynchronizationSpec{
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "Propagate"},
 			},
 			allow: false,
 		}, {
 			name: "Incorrect Role mode",
 			configs: []api.TypeSynchronizationSpec{
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "ignore"},
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "Ignore"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "Propagate"},
 			},
 			allow: false,
 		}, {
 			name: "Incorrect RoleBinding mode",
 			configs: []api.TypeSynchronizationSpec{
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "propagate"},
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "ignore"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "Propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "Ignore"},
 			},
 			allow: false,
 		}, {
 			name: "Duplicate RBAC types with different modes",
 			configs: []api.TypeSynchronizationSpec{
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "Propagate"},
 				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role"},
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "Propagate"},
 			},
 			allow: false,
 		},
 		{
 			name: "Duplicate RBAC types with the same mode",
 			configs: []api.TypeSynchronizationSpec{
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "propagate"},
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "propagate"},
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "Propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "Propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "Propagate"},
 			},
 			allow: false,
 		},
@@ -142,9 +142,9 @@ func TestNonRBACTypes(t *testing.T) {
 		{
 			name: "Correct Non-RBAC types config",
 			configs: []api.TypeSynchronizationSpec{
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "propagate"},
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "propagate"},
-				{APIVersion: "v1", Kind: "Secret", Mode: "ignore"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "Propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "Propagate"},
+				{APIVersion: "v1", Kind: "Secret", Mode: "Ignore"},
 				{APIVersion: "v1", Kind: "ResourceQuota"},
 			},
 			validator: f,
@@ -153,30 +153,30 @@ func TestNonRBACTypes(t *testing.T) {
 		{
 			name: "Resource does not exist",
 			configs: []api.TypeSynchronizationSpec{
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "propagate"},
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "Propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "Propagate"},
 				// "CronTab" kind does not exist in "v1"
-				{APIVersion: "v1", Kind: "CronTab", Mode: "ignore"},
+				{APIVersion: "v1", Kind: "CronTab", Mode: "Ignore"},
 			},
 			validator: f,
 			allow:     false,
 		}, {
 			name: "Duplicate types with different modes",
 			configs: []api.TypeSynchronizationSpec{
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "propagate"},
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "propagate"},
-				{APIVersion: "v1", Kind: "Secret", Mode: "ignore"},
-				{APIVersion: "v1", Kind: "Secret", Mode: "propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "Propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "Propagate"},
+				{APIVersion: "v1", Kind: "Secret", Mode: "Ignore"},
+				{APIVersion: "v1", Kind: "Secret", Mode: "Propagate"},
 			},
 			validator: f,
 			allow:     false,
 		}, {
 			name: "Duplicate types with the same mode",
 			configs: []api.TypeSynchronizationSpec{
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "propagate"},
-				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "propagate"},
-				{APIVersion: "v1", Kind: "Secret", Mode: "ignore"},
-				{APIVersion: "v1", Kind: "Secret", Mode: "ignore"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role", Mode: "Propagate"},
+				{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding", Mode: "Propagate"},
+				{APIVersion: "v1", Kind: "Secret", Mode: "Ignore"},
+				{APIVersion: "v1", Kind: "Secret", Mode: "Ignore"},
 			},
 			validator: f,
 			allow:     false,

diff --git a/incubator/hnc/test/conversion/conversion_test.go b/incubator/hnc/test/conversion/conversion_test.go
@@ -20,6 +20,7 @@ const (
 	// v1alpha1 from CRD status.storedVersions after CRD conversion because it can
 	// be removed only if all the v1alpha1 CRs are reconciled and converted to v1alpha2.
 	crdConversionTime = 7
+	propagationTime = 5
 
 	anchorCRD       = "subnamespaceanchors.hnc.x-k8s.io"
 	hierCRD         = "hierarchyconfigurations.hnc.x-k8s.io"
@@ -133,17 +134,7 @@ spec:
 		// Before conversion, create namespace B with a missing parent A (have to
 		// create A first and then delete it because otherwise the webhook will deny
 		// setting a non-existing namespace as parent).
-		MustRun("kubectl create ns", nsA)
-		MustRun("kubectl create ns", nsB)
-		hierB := `# temp file created by conversion_test.go
-apiVersion: hnc.x-k8s.io/v1alpha1
-kind: HierarchyConfiguration
-metadata:
-  name: hierarchy
-  namespace: e2e-conversion-test-b
-spec:
-  parent: e2e-conversion-test-a`
-		MustApplyYAML(hierB)
+		createSampleV1alpha1Tree()
 		MustRun("kubectl delete ns", nsA)
 		FieldShouldContain(hierCRD, nsB, hierSingleton, ".status.conditions", "CritParentMissing")
 
@@ -167,6 +158,48 @@ spec:
 		FieldShouldContainWithTimeout(configCRD, "", configSingleton, ".apiVersion", "v1alpha2", crdConversionTime)
 		FieldShouldContainMultiple(configCRD, "", configSingleton, ".spec.types", []string{"Role", "RoleBinding"})
 	})
+
+	It("should convert HNCConfig sync modes", func() {
+		// Create a tree with A as the root and B as the child
+		createSampleV1alpha1Tree()
+		// Set "propagate" for Secret in v1alpha1
+		cfg := `# temp file created by conversion_test.go
+apiVersion: hnc.x-k8s.io/v1alpha1
+kind: HNCConfiguration
+metadata:
+  name: config
+spec:
+  types:
+  - apiVersion: rbac.authorization.k8s.io/v1
+    kind: Role
+    mode: propagate
+  - apiVersion: rbac.authorization.k8s.io/v1
+    kind: RoleBinding
+    mode: propagate
+  - apiVersion: v1
+    kind: Secret
+    mode: propagate`
+		MustApplyYAML(cfg)
+		// Create a secret in ns A and make sure it's propagated to ns B.
+		MustRun("kubectl -n", nsA, "create secret generic my-creds-1 --from-literal=password=iama")
+		RunShouldContain("my-creds-1", propagationTime, "kubectl get secrets -n", nsB)
+
+		// Convert
+		setupV1alpha2()
+
+		// Verify CRD conversion.
+		verifyCRDConversion()
+		// Verify sync mode conversion: all three 'propagate' should be converted to 'Propagate'.
+		FieldShouldContain(configCRD, "", configSingleton, ".spec.types", "Propagate")
+		FieldShouldNotContain(configCRD, "", configSingleton, ".spec.types", "propagate")
+		FieldShouldNotContain(configCRD, "", configSingleton, ".spec.types", "Ignore")
+		FieldShouldNotContain(configCRD, "", configSingleton, ".spec.types", "ignore")
+		FieldShouldNotContain(configCRD, "", configSingleton, ".spec.types", "Remove")
+		FieldShouldNotContain(configCRD, "", configSingleton, ".spec.types", "remove")
+		// Verify sync mode behavior.
+		MustRun("kubectl -n", nsA, "create secret generic my-creds-2 --from-literal=password=iama")
+		RunShouldContainMultiple([]string{"my-creds-1", "my-creds-2"}, propagationTime, "kubectl get secrets -n", nsB)
+	})
 })
 
 // Install HNC and kubectl plugin with v1alpha1.
@@ -184,6 +217,10 @@ func setupV1alpha1(hncVersion string){
 // Install HNC and kubectl plugin with v1alpha2.
 func setupV1alpha2(){
 	GinkgoT().Log("Set up v1alpha2")
+	// Delete the deployment to force re-pulling the image. Without this line, a cached
+	// image may be used with the old IfNotPresent imagePullPolicy from 0.5 deployment.
+	// See https://github.com/kubernetes-sigs/multi-tenancy/issues/1025.
+	MustRun("kubectl -n hnc-system delete deployment hnc-controller-manager")
 	MustRun("kubectl apply -f ../../manifests/hnc-manager.yaml")
 	// Wait for the cert rotator to write caBundles in CRD conversion webhooks.
 	ensureCRDConvWHReady()
@@ -220,3 +257,18 @@ func ensureCRDConvWHReady(){
 		RunShouldNotContain("caBundle: Cg==", certsReadyTime, "kubectl get crd "+crd+" -oyaml")
 	}
 }
+
+// createSampleV1alpha1Tree creates a tree with 'a' as the root, 'b' as the child.
+func createSampleV1alpha1Tree(){
+	MustRun("kubectl create ns e2e-conversion-test-a")
+	MustRun("kubectl create ns e2e-conversion-test-b")
+	hierB := `# temp file created by conversion_test.go
+apiVersion: hnc.x-k8s.io/v1alpha1
+kind: HierarchyConfiguration
+metadata:
+  name: hierarchy
+  namespace: e2e-conversion-test-b
+spec:
+  parent: e2e-conversion-test-a`
+	MustApplyYAML(hierB)
+}