From 2a5dacae95dc0b8173a5fcc6601ecf55e5f17b4e Mon Sep 17 00:00:00 2001 From: Drew Walters Date: Fri, 29 Jan 2021 17:55:30 +0000 Subject: [PATCH] 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 Change-Id: I082dac7eae6893decfca12227caf0ea50bc1fa30 --- .../airship.airshipit.org_sipclusters.yaml | 17 +++++-- .../samples/airship_v1beta1_sipcluster.yaml | 46 +++++++++---------- pkg/api/v1/sipcluster_types.go | 43 +++++++++-------- pkg/api/v1/zz_generated.deepcopy.go | 6 +-- pkg/services/set.go | 7 ++- pkg/vbmh/errors.go | 4 +- pkg/vbmh/machines.go | 6 +-- pkg/vbmh/vbmh_test.go | 5 +- testutil/testutil.go | 5 +- 9 files changed, 75 insertions(+), 64 deletions(-) diff --git a/config/crd/bases/airship.airshipit.org_sipclusters.yaml b/config/crd/bases/airship.airshipit.org_sipclusters.yaml index de9e543..ce7d8b4 100644 --- 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 diff --git a/config/samples/airship_v1beta1_sipcluster.yaml b/config/samples/airship_v1beta1_sipcluster.yaml index 9046ae9..11289ff 100644 --- 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 diff --git a/pkg/api/v1/sipcluster_types.go b/pkg/api/v1/sipcluster_types.go index fe6c199..801c462 100644 --- 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"` diff --git a/pkg/api/v1/zz_generated.deepcopy.go b/pkg/api/v1/zz_generated.deepcopy.go index 735a99e..fc0d8d8 100644 --- 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]) } } } diff --git a/pkg/services/set.go b/pkg/services/set.go index 0fdda52..508af38 100644 --- 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 diff --git a/pkg/vbmh/errors.go b/pkg/vbmh/errors.go index 9d24ba5..77f8be7 100644 --- 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 diff --git a/pkg/vbmh/machines.go b/pkg/vbmh/machines.go index 18b5e0d..0895b03 100644 --- 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, } } diff --git a/pkg/vbmh/vbmh_test.go b/pkg/vbmh/vbmh_test.go index 114c02e..13e31e1 100644 --- 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...) diff --git a/testutil/testutil.go b/testutil/testutil.go index 21317ed..14c651c 100644 --- 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, }, }, },