Merge pull request #64 from booxter/main

Fix the defaulting logic for OVNController.spec.external-ids

Makefile

@@ -314,10 +314,14 @@ gowork: ## Generate go.work file to support our multi module repository
 	go work use .
 	go work use ./api
 
+# Skip C003: "Field 'XXX' has both a 'Optional' kubebuilder marker with a
+# default value and an 'omitempty' tag. Either remove the default value or
+# remove 'omitempty'" since spec.external-ids struct in OVNController CRD
+# relies on both being present to fill in the defaults.
 .PHONY: operator-lint
 operator-lint: gowork ## Runs operator-lint
-	GOBIN=$(LOCALBIN) go install github.com/gibizer/operator-lint@v0.3.0
-	go vet -vettool=$(LOCALBIN)/operator-lint ./... ./api/...
+	GOBIN=$(LOCALBIN) go install github.com/gibizer/operator-lint@v0.4.0
+	go vet -vettool=$(LOCALBIN)/operator-lint -C003.skip ./... ./api/...
 
 # Used for webhook testing
 # Please ensure the ovn-controller-manager deployment and

api/bases/ovn.openstack.org_ovncontrollers.yaml

@@ -53,19 +53,17 @@ spec:
                     default: true
                     type: boolean
                   ovn-bridge:
+                    default: br-int
                     type: string
                   ovn-encap-type:
                     default: geneve
-                    description: OvnEncapType - geneve or vxlan
                     enum:
                     - geneve
                     - vxlan
                     type: string
                   system-id:
+                    default: random
                     type: string
-                required:
-                - ovn-bridge
-                - system-id
                 type: object
               networkAttachment:
                 description: NetworkAttachment is a NetworkAttachment resource name

api/v1beta1/ovncontroller_types.go

@@ -38,6 +38,7 @@ const (
 // OVNControllerSpec defines the desired state of OVNController
 type OVNControllerSpec struct {
 	// +kubebuilder:validation:Optional
+	// +kubebuilder:default={}
 	ExternalIDS OVSExternalIDs `json:"external-ids"`
 
 	// +kubebuilder:validation:Required
@@ -132,14 +133,18 @@ func (instance OVNController) IsReady() bool {
 
 // OVSExternalIDs is a set of configuration options for OVS external-ids table
 type OVSExternalIDs struct {
-	SystemID  string `json:"system-id"`
-	OvnBridge string `json:"ovn-bridge"`
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:default="random"
+	SystemID  string `json:"system-id,omitempty"`
+
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:default="br-int"
+	OvnBridge string `json:"ovn-bridge,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default="geneve"
 	// +kubebuilder:validation:Enum={"geneve","vxlan"}
-	// OvnEncapType - geneve or vxlan
-	OvnEncapType string `json:"ovn-encap-type"`
+	OvnEncapType string `json:"ovn-encap-type,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=true

config/crd/bases/ovn.openstack.org_ovncontrollers.yaml

@@ -53,19 +53,17 @@ spec:
                     default: true
                     type: boolean
                   ovn-bridge:
+                    default: br-int
                     type: string
                   ovn-encap-type:
                     default: geneve
-                    description: OvnEncapType - geneve or vxlan
                     enum:
                     - geneve
                     - vxlan
                     type: string
                   system-id:
+                    default: random
                     type: string
-                required:
-                - ovn-bridge
-                - system-id
                 type: object
               networkAttachment:
                 description: NetworkAttachment is a NetworkAttachment resource name

config/samples/ovn_v1beta1_ovncontroller.yaml

@@ -2,8 +2,3 @@ apiVersion: ovn.openstack.org/v1beta1
 kind: OVNController
 metadata:
   name: ovncontroller-sample
-spec:
-  external-ids:
-    system-id: "random"
-    ovn-bridge: "br-int"
-    ovn-encap-type: "geneve"

tests/functional/base_test.go

@@ -175,12 +175,7 @@ func SimulateDaemonsetNumberReady(name types.NamespacedName) {
 }
 
 func GetDefaultOVNControllerSpec() map[string]interface{} {
-	return map[string]interface{}{
-		// Default external Ids not picked up
-		"external-ids": map[string]interface{}{
-			"ovn-encap-type": "geneve",
-		},
-	}
+	return map[string]interface{}{}
 }
 
 func CreateOVNController(namespace string, OVNControllerName string, spec map[string]interface{}) client.Object {
@@ -192,7 +187,9 @@ func CreateOVNController(namespace string, OVNControllerName string, spec map[st
 			"name":      OVNControllerName,
 			"namespace": namespace,
 		},
-		"spec": spec,
+	}
+	if len(spec) != 0 {
+		raw["spec"] = spec
 	}
 	return th.CreateUnstructured(raw)
 }

tests/functional/ovncontroller_controller_test.go

@@ -592,4 +592,25 @@ var _ = Describe("OVNController controller", func() {
 			)
 		})
 	})
+
+	When("OVNController is created with empty spec", func() {
+		var ovnControllerName types.NamespacedName
+
+		BeforeEach(func() {
+			dbs := CreateOVNDBClusters(namespace)
+			DeferCleanup(DeleteOVNDBClusters, dbs)
+			name := fmt.Sprintf("ovn-controller-%s", uuid.New().String())
+			ovnControllerName = types.NamespacedName{Namespace: namespace, Name: name}
+
+			instance := CreateOVNController(namespace, name, map[string]interface{}{})
+			DeferCleanup(th.DeleteInstance, instance)
+		})
+
+		It("applies meaningful defaults", func() {
+			ovnController := GetOVNController(ovnControllerName)
+			Expect(ovnController.Spec.ExternalIDS.OvnEncapType).To(Equal("geneve"))
+			Expect(ovnController.Spec.ExternalIDS.OvnBridge).To(Equal("br-int"))
+			Expect(ovnController.Spec.ExternalIDS.SystemID).To(Equal("random"))
+		})
+	})
 })