Add infra service type validation to SIP CR

Each infrastructure service in the SIPCluster custom resource should be a descendant of a key that is a valid service type; however, errors in the name of an infrastructure service type are only logged as info; processing of the SIP CR continues even when an infrastructure service type is invalid. This change introduces up-front validation by adding a new field to the infrastructure service config, serviceType. When a serviceType is invalid, SIP will not begin processing the CR because the Kubernetes API server will reject the SIPCluster resource. The key under which an infrastructure service resides is now treated as an arbitrary value. Signed-off-by: Drew Walters <andrew.walters@att.com> Change-Id: I082dac7eae6893decfca12227caf0ea50bc1fa30
2021-01-29 17:55:30 +00:00 · 2021-01-29 17:55:30 +00:00 · 2a5dacae95
commit 2a5dacae95
parent 2212b7eaeb
9 changed files with 75 additions and 64 deletions
--- a/config/crd/bases/airship.airshipit.org_sipclusters.yaml
+++ b/config/crd/bases/airship.airshipit.org_sipclusters.yaml
@ -41,7 +41,9 @@ spec:
                with
              type: string
            infra:
-              additionalProperties:
+              description: InfraServices is a list of services that are deployed when
+                a SIPCluster is provisioned.
+              items:
                properties:
                  image:
                    type: string
@ -60,9 +62,18 @@ spec:
                      sshkey:
                        type: string
                    type: object
+                  serviceType:
+                    description: InfraService describes the type of infrastructure
+                      service that should be deployed when a sub-cluster is provisioned.
+                    enum:
+                    - auth
+                    - jumphost
+                    - loadbalancer
+                    type: string
+                required:
+                - serviceType
                type: object
-              description: List of Infrastructure Services
-              type: object
+              type: array
            nodes:
              additionalProperties:
                description: 'NodeSet are the the list of Nodes objects workers, or
--- a/config/samples/airship_v1beta1_sipcluster.yaml
+++ b/config/samples/airship_v1beta1_sipcluster.yaml
@ -12,7 +12,7 @@ spec:
          vm-flavor: 'airshipit.org/vino-flavor=worker'
          spreadTopology: 'per-node' # Support dont'care option.
          count:
-              active: 2 #driven by capi node number
+              active: 1 #driven by capi node number
              standby: 1 #slew for upgrades etc
      master:
          vm-flavor: 'airshipit.org/vino-flavor=master'
@ -21,25 +21,25 @@ spec:
              active: 1
              standby: 1
    infra:
-        loadbalancer:
-          optional:
-            clusterIP: 1.2.3.4 #<-- this aligns to the VIP IP for undercloud k8s
-          image: haproxy:2.3.2
-          nodeLabels:
-              - airship-masters
-          nodePort: 30000
-          nodeInterfaceId: oam-ipv4
-        jumppod:
-          optional:
-            sshKey: rsa.... #<-- this needs to align to the ssh keys provided to cluster api objects
-          image: sshpod:foo
-          nodeLabels:
-            - airship-masters
-          nodePort: 7022
-          nodeInterfaceId: oam-ipv4
-        authpod:
-          image: sshpod:foo
-          nodeLabels:
-            - airship-masters
-          nodePort: 7023
-          nodeInterfaceId: oam-ipv4
+      - serviceType: auth
+        image: sshpod:foo
+        nodeLabels:
+          - airship-masters
+        nodePort: 7023
+        nodeInterfaceId: oam-ipv4
+      - serviceType: jumphost
+        optional:
+          sshKey: rsa.... #<-- this needs to align to the ssh keys provided to cluster api objects
+        image: sshpod:foo
+        nodeLabels:
+          - airship-masters
+        nodePort: 7022
+        nodeInterfaceId: oam-ipv4
+      - serviceType: loadbalancer
+        optional:
+          clusterIP: 1.2.3.4 #<-- this aligns to the VIP IP for undercloud k8s
+        image: haproxy:2.3.2
+        nodeLabels:
+          - airship-masters
+        nodePort: 30000
+        nodeInterfaceId: oam-ipv4
--- a/pkg/api/v1/sipcluster_types.go
+++ b/pkg/api/v1/sipcluster_types.go
@ -44,6 +44,24 @@ type SIPCluster struct {
 	Status SIPClusterStatus `json:"status,omitempty"`
 }

+// Supported infrastructure service types
+// NOTE(drewwalters96): Change the kubebuilder validation comment above the InfraService type when modifying the string
+// values below.
+const (
+	// AuthHostService is an infrastructure service type that represents the sub-cluster authentication service.
+	AuthHostService InfraService = "auth"
+
+	// JumpHostService is an infrastructure service type that represents the sub-cluster jump-host service.
+	JumpHostService InfraService = "jumphost"
+
+	// LoadBalancerService is an infrastructure service type that represents the sub-cluster load balancer service.
+	LoadBalancerService InfraService = "loadbalancer"
+)
+
+// InfraService describes the type of infrastructure service that should be deployed when a sub-cluster is provisioned.
+// +kubebuilder:validation:Enum=auth;jumphost;loadbalancer
+type InfraService string
+
 // SIPClusterSpec defines the desired state of a SIPCluster
 type SIPClusterSpec struct {
 	// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
@ -57,12 +75,8 @@ type SIPClusterSpec struct {
 	// VMRole VMRoles `json:"vm-role,omitempty"`
 	Nodes map[VMRoles]NodeSet `json:"nodes,omitempty"`

-	// Infra is the collection of expeected configuration details
-	// for the multiple infrastructure services or pods that SIP manages
-	//Infra *InfraSet `json:"infra,omitempty"`
-
-	// List of Infrastructure Services
-	InfraServices map[InfraService]InfraConfig `json:"infra"`
+	// InfraServices is a list of services that are deployed when a SIPCluster is provisioned.
+	InfraServices []InfraConfig `json:"infra"`
 }

 // SIPClusterStatus defines the observed state of SIPCluster
@ -98,22 +112,6 @@ const (
 	ReasonTypeReconciliationSucceeded string = "ReconciliationSucceeded"
 )

-// VMRoles defines the states the provisioner will report
-// the tenant has having.
-type InfraService string
-
-// Possible Infra Structure Services
-const (
-	// LoadBalancer Service
-	LoadBalancerService InfraService = "loadbalancer"
-
-	// JumpHostService Service
-	JumpHostService InfraService = "jumppod"
-
-	// AuthHostService Service
-	AuthHostService InfraService = "authpod"
-)
-
 // NodeSet are the the list of Nodes objects workers,
 // or master that definee eexpectations
 // for  the Tenant Clusters
@ -150,6 +148,7 @@ const (
 )

 type InfraConfig struct {
+	ServiceType   InfraService      `json:"serviceType"`
 	OptionalData  *OptsConfig       `json:"optional,omitempty"`
 	Image         string            `json:"image,omitempty"`
 	NodeLabels    map[string]string `json:"nodelabels,omitempty"`
--- a/pkg/api/v1/zz_generated.deepcopy.go
+++ b/pkg/api/v1/zz_generated.deepcopy.go
@ -158,9 +158,9 @@ func (in *SIPClusterSpec) DeepCopyInto(out *SIPClusterSpec) {
 	}
 	if in.InfraServices != nil {
 		in, out := &in.InfraServices, &out.InfraServices
-		*out = make(map[InfraService]InfraConfig, len(*in))
-		for key, val := range *in {
-			(*out)[key] = *val.DeepCopy()
+		*out = make([]InfraConfig, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
 		}
 	}
 }
--- a/pkg/services/set.go
+++ b/pkg/services/set.go
@ -109,10 +109,9 @@ func CreateNS(serviceNamespaceName string, c client.Client) error {
 // ServiceList returns all services defined in Set
 func (ss ServiceSet) ServiceList() []InfraService {
 	var serviceList []InfraService
-	for serviceType, serviceConfig := range ss.sip.Spec.InfraServices {
-		switch serviceType {
+	for _, serviceConfig := range ss.sip.Spec.InfraServices {
+		switch serviceConfig.ServiceType {
 		case airshipv1.LoadBalancerService:
-			ss.logger.Info("Service of type '%s' is defined", "service type", serviceType)
 			serviceList = append(serviceList,
 				newLB(ss.sip.GetName(),
 					ss.sip.Spec.ClusterName,
@ -121,7 +120,7 @@ func (ss ServiceSet) ServiceList() []InfraService {
 					ss.machines,
 					ss.client))
 		default:
-			ss.logger.Info("Service of type '%s' is unknown to SIPCluster controller", "service type", serviceType)
+			ss.logger.Info("serviceType unsupported", "serviceType", serviceConfig.ServiceType)
 		}
 	}
 	return serviceList
--- a/pkg/vbmh/errors.go
+++ b/pkg/vbmh/errors.go
@ -28,14 +28,14 @@ func (e ErrorUnableToFullySchedule) Error() string {

 type ErrorHostIPNotFound struct {
 	HostName    string
-	ServiceName airshipv1.InfraService
+	ServiceType airshipv1.InfraService
 	IPInterface string
 	Message     string
 }

 func (e ErrorHostIPNotFound) Error() string {
 	return fmt.Sprintf("Unable to identify the vBMH Host %v IP address on interface %v required by "+
-		"Infrastructure Service %v %s ", e.HostName, e.IPInterface, e.ServiceName, e.Message)
+		"Infrastructure Service %v %s", e.HostName, e.IPInterface, e.ServiceType, e.Message)
 }

 // ErrorUknownSpreadTopology is returned when wrong AuthType is provided
--- a/pkg/vbmh/machines.go
+++ b/pkg/vbmh/machines.go
@ -582,12 +582,12 @@ func (ml *MachineList) Extrapolate(sip airshipv1.SIPCluster, c client.Client) bo
 ***/

 func (ml *MachineList) getIP(machine *Machine, networkDataSecret *corev1.Secret,
-	infraServices map[airshipv1.InfraService]airshipv1.InfraConfig) error {
+	infraServices []airshipv1.InfraConfig) error {
 	var secretData interface{}
 	// Now I have the Secret
 	// Lets find the IP's for all Interfaces defined in Cfg
 	foundIP := false
-	for svcName, svcCfg := range infraServices {
+	for _, svcCfg := range infraServices {
 		// Did I already find the IP for these interface
 		if machine.Data.IPOnInterface[svcCfg.NodeInterface] == "" {
 			err := json.Unmarshal(networkDataSecret.Data["networkData"], &secretData)
@ -616,7 +616,7 @@ func (ml *MachineList) getIP(machine *Machine, networkDataSecret *corev1.Secret,
 		if !foundIP {
 			return &ErrorHostIPNotFound{
 				HostName:    machine.BMH.ObjectMeta.Name,
-				ServiceName: svcName,
+				ServiceType: svcCfg.ServiceType,
 				IPInterface: svcCfg.NodeInterface,
 			}
 		}
--- a/pkg/vbmh/vbmh_test.go
+++ b/pkg/vbmh/vbmh_test.go
@ -114,14 +114,15 @@ var _ = Describe("MachineList", func() {
 		}

 		sipCluster := testutil.CreateSIPCluster("subcluster-1", "default", 1, 3)
-		sipCluster.Spec.InfraServices = map[airshipv1.InfraService]airshipv1.InfraConfig{
-			airshipv1.LoadBalancerService: {
+		sipCluster.Spec.InfraServices = []airshipv1.InfraConfig{
+			{
 				Image: "haproxy:latest",
 				NodeLabels: map[string]string{
 					"test": "true",
 				},
 				NodePort:      30000,
 				NodeInterface: "oam-ipv4",
+				ServiceType:   airshipv1.LoadBalancerService,
 			},
 		}
 		k8sClient := mockClient.NewFakeClient(objs...)
--- a/testutil/testutil.go
+++ b/testutil/testutil.go
@ -229,10 +229,11 @@ func CreateSIPCluster(name string, namespace string, masters int, workers int) *
 					},
 				},
 			},
-			InfraServices: map[airshipv1.InfraService]airshipv1.InfraConfig{
-				airshipv1.LoadBalancerService: {
+			InfraServices: []airshipv1.InfraConfig{
+				{
 					NodeInterface: "eno3",
 					NodePort:      30000,
+					ServiceType:   airshipv1.LoadBalancerService,
 				},
 			},
 		},