From 5b20b10fc2d8acf1988f17f0fead20ac3262bac7 Mon Sep 17 00:00:00 2001 From: Mridul Date: Tue, 19 Sep 2023 11:15:17 +0530 Subject: [PATCH 01/13] added slicegatewayServiceType field Signed-off-by: Mridul --- apis/controller/v1alpha1/sliceconfig_types.go | 10 +++++++++ .../v1alpha1/zz_generated.deepcopy.go | 22 ++++++++++++++++++- .../v1alpha1/workersliceconfig_types.go | 2 ++ .../controller.kubeslice.io_sliceconfigs.yaml | 12 ++++++++++ ...orker.kubeslice.io_workersliceconfigs.yaml | 5 +++++ service/kube_slice_resource_names.go | 7 +++--- service/worker_slice_config_service.go | 19 +++++++++++++++- 7 files changed, 72 insertions(+), 5 deletions(-) diff --git a/apis/controller/v1alpha1/sliceconfig_types.go b/apis/controller/v1alpha1/sliceconfig_types.go index e6b8e868..999a757a 100644 --- a/apis/controller/v1alpha1/sliceconfig_types.go +++ b/apis/controller/v1alpha1/sliceconfig_types.go @@ -74,6 +74,16 @@ type WorkerSliceGatewayProvider struct { //+kubebuilder:default:=Local // +kubebuilder:validation:Required SliceCaType string `json:"sliceCaType"` + + SliceGatewayServiceType []SliceGatewayServiceType `json:"sliceGatewayServiceType,omitempty"` +} + +type SliceGatewayServiceType struct { + // +kubebuilder:validation:Required + Cluster string `json:"cluster,omitempty"` + // +kubebuilder:validation:Required + //+kubebuilder:validation:Enum:=NodePort;LoadBalancer + Type string `json:"type,omitempty"` } // QOSProfile is the QOS Profile configuration from backend diff --git a/apis/controller/v1alpha1/zz_generated.deepcopy.go b/apis/controller/v1alpha1/zz_generated.deepcopy.go index 958b7b9c..5c3ebba4 100644 --- a/apis/controller/v1alpha1/zz_generated.deepcopy.go +++ b/apis/controller/v1alpha1/zz_generated.deepcopy.go @@ -674,7 +674,7 @@ func (in *SliceConfigList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SliceConfigSpec) DeepCopyInto(out *SliceConfigSpec) { *out = *in - out.SliceGatewayProvider = in.SliceGatewayProvider + in.SliceGatewayProvider.DeepCopyInto(&out.SliceGatewayProvider) if in.Clusters != nil { in, out := &in.Clusters, &out.Clusters *out = make([]string, len(*in)) @@ -736,6 +736,21 @@ func (in *SliceConfigStatus) DeepCopy() *SliceConfigStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SliceGatewayServiceType) DeepCopyInto(out *SliceGatewayServiceType) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SliceGatewayServiceType. +func (in *SliceGatewayServiceType) DeepCopy() *SliceGatewayServiceType { + if in == nil { + return nil + } + out := new(SliceGatewayServiceType) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SliceNamespaceSelection) DeepCopyInto(out *SliceNamespaceSelection) { *out = *in @@ -1051,6 +1066,11 @@ func (in *VpnKeyRotationStatus) DeepCopy() *VpnKeyRotationStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *WorkerSliceGatewayProvider) DeepCopyInto(out *WorkerSliceGatewayProvider) { *out = *in + if in.SliceGatewayServiceType != nil { + in, out := &in.SliceGatewayServiceType, &out.SliceGatewayServiceType + *out = make([]SliceGatewayServiceType, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkerSliceGatewayProvider. diff --git a/apis/worker/v1alpha1/workersliceconfig_types.go b/apis/worker/v1alpha1/workersliceconfig_types.go index 5a99981c..531211ff 100644 --- a/apis/worker/v1alpha1/workersliceconfig_types.go +++ b/apis/worker/v1alpha1/workersliceconfig_types.go @@ -59,6 +59,8 @@ type WorkerSliceGatewayProvider struct { SliceGatewayType string `json:"sliceGatewayType,omitempty"` //+kubebuilder:default:=Local SliceCaType string `json:"sliceCaType,omitempty"` + //+kubebuilder:validation:Enum:=NodePort;LoadBalancer + SliceGatewayServiceType string `json:"sliceGatewayServiceType,omitempty"` } // QOSProfile is the QOS Profile configuration from backend diff --git a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml index 4d297ef9..02ed58c4 100644 --- a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml +++ b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml @@ -159,6 +159,18 @@ spec: sliceCaType: default: Local type: string + sliceGatewayServiceType: + items: + properties: + cluster: + type: string + type: + enum: + - NodePort + - LoadBalancer + type: string + type: object + type: array sliceGatewayType: default: OpenVPN type: string diff --git a/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml b/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml index 916eb71b..1542d9b9 100644 --- a/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml +++ b/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml @@ -117,6 +117,11 @@ spec: sliceCaType: default: Local type: string + sliceGatewayServiceType: + enum: + - NodePort + - LoadBalancer + type: string sliceGatewayType: default: OpenVPN type: string diff --git a/service/kube_slice_resource_names.go b/service/kube_slice_resource_names.go index d567df97..03932de3 100644 --- a/service/kube_slice_resource_names.go +++ b/service/kube_slice_resource_names.go @@ -151,9 +151,10 @@ var ( ) const ( - serverGateway = "Server" - clientGateway = "Client" - workerSliceGatewayType = "OpenVPN" + serverGateway = "Server" + clientGateway = "Client" + workerSliceGatewayType = "OpenVPN" + defaultSliceGatewayServiceType = "NodePort" ) var ( diff --git a/service/worker_slice_config_service.go b/service/worker_slice_config_service.go index 19f30890..b914e881 100644 --- a/service/worker_slice_config_service.go +++ b/service/worker_slice_config_service.go @@ -19,9 +19,10 @@ package service import ( "context" "fmt" - "github.com/kubeslice/kubeslice-controller/metrics" "time" + "github.com/kubeslice/kubeslice-controller/metrics" + "github.com/kubeslice/kubeslice-controller/events" "go.uber.org/zap" @@ -211,6 +212,22 @@ outer: logger.With(zap.Error(err)).Errorf("Failed to deep copy external gateway configuration") } + // Reconcile Slice gateway service type + sliceGatewayProvider := workerv1alpha1.WorkerSliceGatewayProvider{ + SliceGatewayType: sliceConfig.Spec.SliceGatewayProvider.SliceGatewayType, + SliceCaType: sliceConfig.Spec.SliceGatewayProvider.SliceCaType, + } + gwSvcTypePresent := false + for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { + if gwSvcType.Cluster == "*" || gwSvcType.Cluster == workerSliceConfig.Labels["worker-cluster"] { + sliceGatewayProvider.SliceGatewayServiceType = gwSvcType.Type + gwSvcTypePresent = true + } + } + if !gwSvcTypePresent { + sliceGatewayProvider.SliceGatewayServiceType = defaultSliceGatewayServiceType + } + // Reconcile the Namespace Isolation Profile controllerIsolationProfile := sliceConfig.Spec.NamespaceIsolationProfile workerIsolationProfile := workerv1alpha1.NamespaceIsolationProfile{ From 7180b8edb2b50aebdb26a702ec4c291a5c9ba058 Mon Sep 17 00:00:00 2001 From: Mridul Date: Mon, 25 Sep 2023 21:38:42 +0530 Subject: [PATCH 02/13] added slice config validation Signed-off-by: Mridul --- apis/controller/v1alpha1/sliceconfig_types.go | 4 +-- .../controller.kubeslice.io_sliceconfigs.yaml | 3 ++ service/slice_config_webhook_validation.go | 28 +++++++++++++++++++ service/worker_slice_config_service.go | 1 + 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/apis/controller/v1alpha1/sliceconfig_types.go b/apis/controller/v1alpha1/sliceconfig_types.go index 999a757a..e0fa2edc 100644 --- a/apis/controller/v1alpha1/sliceconfig_types.go +++ b/apis/controller/v1alpha1/sliceconfig_types.go @@ -80,10 +80,10 @@ type WorkerSliceGatewayProvider struct { type SliceGatewayServiceType struct { // +kubebuilder:validation:Required - Cluster string `json:"cluster,omitempty"` + Cluster string `json:"cluster"` // +kubebuilder:validation:Required //+kubebuilder:validation:Enum:=NodePort;LoadBalancer - Type string `json:"type,omitempty"` + Type string `json:"type"` } // QOSProfile is the QOS Profile configuration from backend diff --git a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml index 02ed58c4..e7c956da 100644 --- a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml +++ b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml @@ -169,6 +169,9 @@ spec: - NodePort - LoadBalancer type: string + required: + - cluster + - type type: object type: array sliceGatewayType: diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index 5177928d..84b9683c 100644 --- a/service/slice_config_webhook_validation.go +++ b/service/slice_config_webhook_validation.go @@ -49,6 +49,9 @@ func ValidateSliceConfigCreate(ctx context.Context, sliceConfig *controllerv1alp if err := validateClustersOnCreate(ctx, sliceConfig); err != nil { return apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "SliceConfig"}, sliceConfig.Name, field.ErrorList{err}) } + if err := validateSlicegatewayServiceType(ctx, sliceConfig); err != nil { + return apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "SliceConfig"}, sliceConfig.Name, field.ErrorList{err}) + } if err := validateQosProfile(ctx, sliceConfig); err != nil { return apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "SliceConfig"}, sliceConfig.Name, field.ErrorList{err}) } @@ -78,6 +81,9 @@ func ValidateSliceConfigUpdate(ctx context.Context, sliceConfig *controllerv1alp if err := validateClustersOnUpdate(ctx, sliceConfig, old); err != nil { return apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "SliceConfig"}, sliceConfig.Name, field.ErrorList{err}) } + if err := validateSlicegatewayServiceType(ctx, sliceConfig); err != nil { + return apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "SliceConfig"}, sliceConfig.Name, field.ErrorList{err}) + } if err := validateQosProfile(ctx, sliceConfig); err != nil { return apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "SliceConfig"}, sliceConfig.Name, field.ErrorList{err}) } @@ -330,6 +336,28 @@ func validateClustersOnUpdate(ctx context.Context, sliceConfig *controllerv1alph return nil } +// to validate the SlicegatewayServiceType array +func validateSlicegatewayServiceType(ctx context.Context, sliceConfig *controllerv1alpha1.SliceConfig) *field.Error { + freq := make(map[string]int) + for _, sliceGwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { + cluster := sliceGwSvcType.Cluster + freq[cluster] += 1 + // cluster name can't be empty + if cluster == "" { + return field.Invalid(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType").Child("Cluster"), cluster, "Cluster name can't be empty") + } + // cluster should participate in slice + if cluster != "*" && !util.ContainsString(sliceConfig.Spec.Clusters, cluster) { + return field.Invalid(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType").Child("Cluster"), cluster, "Cluster is not participating in slice config") + } + // don't allow duplicate cluster values + if freq[cluster] > 1 { + return field.Invalid(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType").Child("Cluster"), cluster, "Duplicate entries are not allowed") + } + } + return nil +} + // preventUpdate is a function to stop/avoid the update of config of slice func preventUpdate(ctx context.Context, sc *controllerv1alpha1.SliceConfig, old runtime.Object) *field.Error { sliceConfig := old.(*controllerv1alpha1.SliceConfig) diff --git a/service/worker_slice_config_service.go b/service/worker_slice_config_service.go index b914e881..cec896a8 100644 --- a/service/worker_slice_config_service.go +++ b/service/worker_slice_config_service.go @@ -254,6 +254,7 @@ outer: } workerSliceConfig.Spec.ExternalGatewayConfig = externalGatewayConfig + workerSliceConfig.Spec.SliceGatewayProvider = sliceGatewayProvider workerSliceConfig.Spec.NamespaceIsolationProfile = workerIsolationProfile workerSliceConfig.Spec.SliceName = sliceConfig.Name workerSliceConfig.Spec.Octet = octet From 3b9031a8548a40d4753b781df0ae8372d96d5213 Mon Sep 17 00:00:00 2001 From: Mridul Date: Tue, 26 Sep 2023 19:11:57 +0530 Subject: [PATCH 03/13] workerslicegw change: adding gw connectivity type Signed-off-by: Mridul --- apis/controller/v1alpha1/sliceconfig_types.go | 1 + .../v1alpha1/workersliceconfig_types.go | 1 + .../v1alpha1/workerslicegateway_types.go | 30 +++++++++++-------- apis/worker/v1alpha1/zz_generated.deepcopy.go | 5 ++++ .../controller.kubeslice.io_sliceconfigs.yaml | 1 + ...orker.kubeslice.io_workersliceconfigs.yaml | 1 + ...rker.kubeslice.io_workerslicegateways.yaml | 14 +++++++++ service/worker_slice_config_service.go | 4 ++- service/worker_slice_gateway_service.go | 15 ++++++++++ 9 files changed, 58 insertions(+), 14 deletions(-) diff --git a/apis/controller/v1alpha1/sliceconfig_types.go b/apis/controller/v1alpha1/sliceconfig_types.go index e0fa2edc..1a80d105 100644 --- a/apis/controller/v1alpha1/sliceconfig_types.go +++ b/apis/controller/v1alpha1/sliceconfig_types.go @@ -82,6 +82,7 @@ type SliceGatewayServiceType struct { // +kubebuilder:validation:Required Cluster string `json:"cluster"` // +kubebuilder:validation:Required + //+kubebuilder:default:=NodePort //+kubebuilder:validation:Enum:=NodePort;LoadBalancer Type string `json:"type"` } diff --git a/apis/worker/v1alpha1/workersliceconfig_types.go b/apis/worker/v1alpha1/workersliceconfig_types.go index 531211ff..55125e1f 100644 --- a/apis/worker/v1alpha1/workersliceconfig_types.go +++ b/apis/worker/v1alpha1/workersliceconfig_types.go @@ -59,6 +59,7 @@ type WorkerSliceGatewayProvider struct { SliceGatewayType string `json:"sliceGatewayType,omitempty"` //+kubebuilder:default:=Local SliceCaType string `json:"sliceCaType,omitempty"` + //+kubebuilder:default:=NodePort //+kubebuilder:validation:Enum:=NodePort;LoadBalancer SliceGatewayServiceType string `json:"sliceGatewayServiceType,omitempty"` } diff --git a/apis/worker/v1alpha1/workerslicegateway_types.go b/apis/worker/v1alpha1/workerslicegateway_types.go index a0f5db65..82820e62 100644 --- a/apis/worker/v1alpha1/workerslicegateway_types.go +++ b/apis/worker/v1alpha1/workerslicegateway_types.go @@ -29,23 +29,27 @@ type WorkerSliceGatewaySpec struct { //+kubebuilder:default:=OpenVPN GatewayType string `json:"gatewayType,omitempty"` //+kubebuilder:validation:Enum:=Client;Server - GatewayHostType string `json:"gatewayHostType,omitempty"` - GatewayCredentials GatewayCredentials `json:"gatewayCredentials,omitempty"` - LocalGatewayConfig SliceGatewayConfig `json:"localGatewayConfig,omitempty"` - RemoteGatewayConfig SliceGatewayConfig `json:"remoteGatewayConfig,omitempty"` - GatewayNumber int `json:"gatewayNumber,omitempty"` + GatewayHostType string `json:"gatewayHostType,omitempty"` + //+kubebuilder:default:=NodePort + //+kubebuilder:validation:Enum:=NodePort;LoadBalancer + GatewayConnectivityType string `json:"gatewayConnectivityType,omitempty"` + GatewayCredentials GatewayCredentials `json:"gatewayCredentials,omitempty"` + LocalGatewayConfig SliceGatewayConfig `json:"localGatewayConfig,omitempty"` + RemoteGatewayConfig SliceGatewayConfig `json:"remoteGatewayConfig,omitempty"` + GatewayNumber int `json:"gatewayNumber,omitempty"` } type SliceGatewayConfig struct { //+kubebuilder:deprecatedversion:warning="worker/v1alpha1 NodeIp is deprecated...use NodeIps" - NodeIp string `json:"nodeIp,omitempty"` - NodeIps []string `json:"nodeIps,omitempty"` - NodePort int `json:"nodePort,omitempty"` - NodePorts []int `json:"nodePorts,omitempty"` - GatewayName string `json:"gatewayName,omitempty"` - ClusterName string `json:"clusterName,omitempty"` - VpnIp string `json:"vpnIp,omitempty"` - GatewaySubnet string `json:"gatewaySubnet,omitempty"` + NodeIp string `json:"nodeIp,omitempty"` + NodeIps []string `json:"nodeIps,omitempty"` + LoadBalancerIps []string `json:"loadBalancerIps,omitempty"` + NodePort int `json:"nodePort,omitempty"` + NodePorts []int `json:"nodePorts,omitempty"` + GatewayName string `json:"gatewayName,omitempty"` + ClusterName string `json:"clusterName,omitempty"` + VpnIp string `json:"vpnIp,omitempty"` + GatewaySubnet string `json:"gatewaySubnet,omitempty"` } type GatewayCredentials struct { diff --git a/apis/worker/v1alpha1/zz_generated.deepcopy.go b/apis/worker/v1alpha1/zz_generated.deepcopy.go index 603e4106..51f6b2f4 100644 --- a/apis/worker/v1alpha1/zz_generated.deepcopy.go +++ b/apis/worker/v1alpha1/zz_generated.deepcopy.go @@ -226,6 +226,11 @@ func (in *SliceGatewayConfig) DeepCopyInto(out *SliceGatewayConfig) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.LoadBalancerIps != nil { + in, out := &in.LoadBalancerIps, &out.LoadBalancerIps + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.NodePorts != nil { in, out := &in.NodePorts, &out.NodePorts *out = make([]int, len(*in)) diff --git a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml index e7c956da..39adb202 100644 --- a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml +++ b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml @@ -165,6 +165,7 @@ spec: cluster: type: string type: + default: NodePort enum: - NodePort - LoadBalancer diff --git a/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml b/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml index 1542d9b9..e497fdb3 100644 --- a/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml +++ b/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml @@ -118,6 +118,7 @@ spec: default: Local type: string sliceGatewayServiceType: + default: NodePort enum: - NodePort - LoadBalancer diff --git a/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml b/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml index b9a7c188..fc78c8c7 100644 --- a/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml +++ b/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml @@ -35,6 +35,12 @@ spec: spec: description: WorkerSliceGatewaySpec defines the desired state of WorkerSliceGateway properties: + gatewayConnectivityType: + default: NodePort + enum: + - NodePort + - LoadBalancer + type: string gatewayCredentials: properties: secretName: @@ -58,6 +64,10 @@ spec: type: string gatewaySubnet: type: string + loadBalancerIps: + items: + type: string + type: array nodeIp: type: string nodeIps: @@ -81,6 +91,10 @@ spec: type: string gatewaySubnet: type: string + loadBalancerIps: + items: + type: string + type: array nodeIp: type: string nodeIps: diff --git a/service/worker_slice_config_service.go b/service/worker_slice_config_service.go index cec896a8..94ce1a73 100644 --- a/service/worker_slice_config_service.go +++ b/service/worker_slice_config_service.go @@ -37,6 +37,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +const workerSliceConfigNameFormat = "%s-%s" + type IWorkerSliceConfigService interface { ReconcileWorkerSliceConfig(ctx context.Context, req ctrl.Request) (ctrl.Result, error) DeleteWorkerSliceConfigByLabel(ctx context.Context, label map[string]string, namespace string) error @@ -293,7 +295,7 @@ func (s *WorkerSliceConfigService) CreateMinimalWorkerSliceConfig(ctx context.Co clusterMap := s.ComputeClusterMap(clusters, workerSliceConfigs) for _, cluster := range clusters { logger.Debugf("Cluster Object %s", cluster) - workerSliceConfigName := fmt.Sprintf("%s-%s", name, cluster) + workerSliceConfigName := fmt.Sprintf(workerSliceConfigNameFormat, name, cluster) existingSlice := &workerv1alpha1.WorkerSliceConfig{} found, err := util.GetResourceIfExist(ctx, client.ObjectKey{ Name: workerSliceConfigName, diff --git a/service/worker_slice_gateway_service.go b/service/worker_slice_gateway_service.go index 8f1cd7a0..8dad6c96 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -207,6 +207,21 @@ func (s *WorkerSliceGatewayService) ReconcileWorkerSliceGateways(ctx context.Con logger.Infof("sliceConfig %v not found, returning from reconciler loop.", req.NamespacedName) return ctrl.Result{}, nil } + // reconcile gateway connectivity type + var clusterName string + if workerSliceGateway.Spec.GatewayHostType == serverGateway { + clusterName = workerSliceGateway.Labels["worker-cluster"] + } else { + clusterName = workerSliceGateway.Labels["remote-cluster"] + } + gatewayConnectivityType := defaultSliceGatewayServiceType + for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { + if gwSvcType.Cluster == "*" || gwSvcType.Cluster == clusterName { + gatewayConnectivityType = gwSvcType.Type + } + } + workerSliceGateway.Spec.GatewayConnectivityType = gatewayConnectivityType + logger.Debugf("setting gwsvctype %s", workerSliceGateway.Spec.GatewayConnectivityType) workerSliceGateway.Spec.GatewayType = workerSliceGatewayType workerSliceGateway.UID = "" err = util.UpdateResource(ctx, workerSliceGateway) From baffde35c1a31dd62b664070e47706b0745c80c7 Mon Sep 17 00:00:00 2001 From: Mridul Date: Tue, 3 Oct 2023 22:41:58 +0530 Subject: [PATCH 04/13] feat(AM-11526): reconcile logic for LB IPs Signed-off-by: Mridul --- service/worker_slice_gateway_service.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/service/worker_slice_gateway_service.go b/service/worker_slice_gateway_service.go index 8dad6c96..66e60171 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -249,11 +249,16 @@ func (s *WorkerSliceGatewayService) reconcileNodeIPAndNodePort(ctx context.Conte if !reflect.DeepEqual(localGateway.Spec.LocalGatewayConfig.NodeIps, remoteGateway.Spec.RemoteGatewayConfig.NodeIps) || !reflect.DeepEqual(localGateway.Spec.LocalGatewayConfig.NodeIp, remoteGateway.Spec.RemoteGatewayConfig.NodeIp) || localGateway.Spec.LocalGatewayConfig.NodePort != remoteGateway.Spec.RemoteGatewayConfig.NodePort || - !reflect.DeepEqual(localGateway.Spec.LocalGatewayConfig.NodePorts, remoteGateway.Spec.RemoteGatewayConfig.NodePorts) { + !reflect.DeepEqual(localGateway.Spec.LocalGatewayConfig.NodePorts, remoteGateway.Spec.RemoteGatewayConfig.NodePorts) || + (localGateway.Spec.GatewayHostType == serverGateway && + !reflect.DeepEqual(localGateway.Spec.LocalGatewayConfig.LoadBalancerIps, remoteGateway.Spec.RemoteGatewayConfig.LoadBalancerIps)) { + remoteGateway.Spec.RemoteGatewayConfig.NodeIp = localGateway.Spec.LocalGatewayConfig.NodeIp remoteGateway.Spec.RemoteGatewayConfig.NodeIps = localGateway.Spec.LocalGatewayConfig.NodeIps remoteGateway.Spec.RemoteGatewayConfig.NodePort = localGateway.Spec.LocalGatewayConfig.NodePort remoteGateway.Spec.RemoteGatewayConfig.NodePorts = localGateway.Spec.LocalGatewayConfig.NodePorts + remoteGateway.Spec.RemoteGatewayConfig.LoadBalancerIps = localGateway.Spec.LocalGatewayConfig.LoadBalancerIps + err = util.UpdateResource(ctx, &remoteGateway) if err != nil { return err From aec07c41cc3da349921374b2dab55fc8549fbfc9 Mon Sep 17 00:00:00 2001 From: Mridul Date: Tue, 17 Oct 2023 13:25:52 +0530 Subject: [PATCH 05/13] feat(AM-11526): updating gwConnType is not allowed Signed-off-by: Mridul --- service/slice_config_webhook_validation.go | 17 ++++++++++- .../slice_config_webhook_validation_test.go | 29 +++++++++++++++++++ .../worker_slice_config_webhook_validation.go | 4 +++ ...er_slice_config_webhook_validation_test.go | 22 +++++++++++++- ...worker_slice_gateway_webhook_validation.go | 4 +++ ...r_slice_gateway_webhook_validation_test.go | 22 +++++++++++++- 6 files changed, 95 insertions(+), 3 deletions(-) diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index 84b9683c..e313d3cf 100644 --- a/service/slice_config_webhook_validation.go +++ b/service/slice_config_webhook_validation.go @@ -382,7 +382,22 @@ func preventUpdate(ctx context.Context, sc *controllerv1alpha1.SliceConfig, old return field.Invalid(field.NewPath("Spec").Child("VPNConfig").Child("Cipher"), sc.Spec.VPNConfig.Cipher, "cannot be updated") } } - + // can't switch gw svc types + gwSvcType := map[string]string{} + // create cluster:GwType map from old config + for _, i := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { + gwSvcType[i.Cluster] = i.Type + } + // check new config + for _, new := range sc.Spec.SliceGatewayProvider.SliceGatewayServiceType { + oldType, exists := gwSvcType[new.Cluster] + if exists && new.Type != oldType { + return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed") + } + if !exists && new.Type != defaultSliceGatewayServiceType { + return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed") + } + } return nil } diff --git a/service/slice_config_webhook_validation_test.go b/service/slice_config_webhook_validation_test.go index 76dffc4a..5ba28282 100644 --- a/service/slice_config_webhook_validation_test.go +++ b/service/slice_config_webhook_validation_test.go @@ -89,6 +89,7 @@ var SliceConfigWebhookValidationTestBed = map[string]func(*testing.T){ "SliceConfigWebhookValidation_UpdateValidateSliceConfigWithExternalGatewayConfigHasAsterisksInMoreThanOnePlace": UpdateValidateSliceConfigWithExternalGatewayConfigHasAsterisksInMoreThanOnePlace, "SliceConfigWebhookValidation_UpdateValidateSliceConfigWithExternalGatewayConfigHasDuplicateClusters": UpdateValidateSliceConfigWithExternalGatewayConfigHasDuplicateClusters, "SliceConfigWebhookValidation_UpdateValidateSliceConfigWithoutErrors": UpdateValidateSliceConfigWithoutErrors, + "SliceConfigWebhookValidation_UpdateValidateSliceGatewayServiceType": UpdateValidateSliceConfig_SliceGatewayServiceType, "SliceConfigWebhookValidation_DeleteValidateSliceConfigWithApplicationNamespacesNotEmpty": DeleteValidateSliceConfigWithApplicationNamespacesAndAllowedNamespacesNotEmpty, "SliceConfigWebhookValidation_DeleteValidateSliceConfigWithOnboardedAppNamespacesNotEmpty": DeleteValidateSliceConfigWithOnboardedAppNamespacesNotEmpty, "SliceConfigWebhookValidation_validateAllowedNamespacesWithDuplicateClusters": ValidateAllowedNamespacesWithDuplicateClusters, @@ -119,6 +120,34 @@ var SliceConfigWebhookValidationTestBed = map[string]func(*testing.T){ "SliceConfigWebhookValidation_UpdateValidateSliceConfigUpdatingVPNCipher": UpdateValidateSliceConfigUpdatingVPNCipher, } +func UpdateValidateSliceConfig_SliceGatewayServiceType(t *testing.T) { + name := "test-slice" + namespace := "demons" + oldSliceConfig := controllerv1alpha1.SliceConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } + oldSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ + { + Cluster: "c1", + Type: "LoadBalancer", + }, + } + clientMock, newSliceConfig, ctx := setupSliceConfigWebhookValidationTest(name, namespace) + newSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ + { + Cluster: "c1", + Type: "NodePort", + }, + } + err := ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig)) + require.NotNil(t, err) + require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") + require.Contains(t, err.Error(), "update not allowed") + clientMock.AssertExpectations(t) +} func CreateValidateProjectNamespaceDoesNotExist(t *testing.T) { name := "slice_config" namespace := "namespace" diff --git a/service/worker_slice_config_webhook_validation.go b/service/worker_slice_config_webhook_validation.go index f79ed38a..e2359e81 100644 --- a/service/worker_slice_config_webhook_validation.go +++ b/service/worker_slice_config_webhook_validation.go @@ -18,6 +18,7 @@ package service import ( "context" + "k8s.io/apimachinery/pkg/runtime" workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" @@ -40,5 +41,8 @@ func preventUpdateWorkerSliceConfig(ctx context.Context, ss *workerv1alpha1.Work if workerSliceConfig.Spec.Octet != nil && *workerSliceConfig.Spec.Octet != *ss.Spec.Octet { return field.Invalid(field.NewPath("Spec").Child("Octet"), *ss.Spec.Octet, "cannot be updated") } + if workerSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType != ss.Spec.SliceGatewayProvider.SliceGatewayType { + return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed") + } return nil } diff --git a/service/worker_slice_config_webhook_validation_test.go b/service/worker_slice_config_webhook_validation_test.go index 39114ad9..ea899835 100644 --- a/service/worker_slice_config_webhook_validation_test.go +++ b/service/worker_slice_config_webhook_validation_test.go @@ -18,9 +18,10 @@ package service import ( "context" - "k8s.io/apimachinery/pkg/runtime" "testing" + "k8s.io/apimachinery/pkg/runtime" + "github.com/dailymotion/allure-go" workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" "github.com/kubeslice/kubeslice-controller/util" @@ -43,6 +44,25 @@ func TestWorkerSliceConfigWebhookValidationSuite(t *testing.T) { var WorkerSliceConfigWebhookValidationTestBed = map[string]func(*testing.T){ "WorkerSliceConfigWebhookValidation_UpdateValidateWorkerSliceConfigUpdatingOctet": UpdateValidateWorkerSliceConfigUpdatingOctet, "WorkerSliceConfigWebhookValidation_UpdateValidateWorkerSliceConfigWithoutErrors": UpdateValidateWorkerSliceConfigWithoutErrors, + "WorkerSliceConfigWebhookValidation_UpdateSliceGatewayServiceType": UpdateSliceGatewayServiceType, +} + +func UpdateSliceGatewayServiceType(t *testing.T) { + name := "slice-clusterx" + namespace := "demons" + clientMock, newWorkerSliceConfig, ctx := setupWorkerSliceConfigWebhookValidationTest(name, namespace) + existingWorkerSliceConfig := workerv1alpha1.WorkerSliceConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } + existingWorkerSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = "LoadBalancer" + newWorkerSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = "NodePort" + err := ValidateWorkerSliceConfigUpdate(ctx, &existingWorkerSliceConfig, runtime.Object(newWorkerSliceConfig)) + require.NotNil(t, err) + require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") + clientMock.AssertExpectations(t) } func UpdateValidateWorkerSliceConfigUpdatingOctet(t *testing.T) { diff --git a/service/worker_slice_gateway_webhook_validation.go b/service/worker_slice_gateway_webhook_validation.go index 4e94ccdf..9685159e 100644 --- a/service/worker_slice_gateway_webhook_validation.go +++ b/service/worker_slice_gateway_webhook_validation.go @@ -18,6 +18,7 @@ package service import ( "context" + "k8s.io/apimachinery/pkg/runtime" workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" @@ -40,5 +41,8 @@ func preventUpdateWorkerSliceGateway(workerSliceGatewayCtx context.Context, sg * if workerSliceGateway.Spec.GatewayNumber != sg.Spec.GatewayNumber { return field.Invalid(field.NewPath("Spec").Child("GatewayNumber"), sg.Spec.GatewayNumber, "cannot be updated") } + if workerSliceGateway.Spec.GatewayConnectivityType != sg.Spec.GatewayConnectivityType { + return field.Forbidden(field.NewPath("Spec").Child("GatewayConnectivityType"), "update not allowed") + } return nil } diff --git a/service/worker_slice_gateway_webhook_validation_test.go b/service/worker_slice_gateway_webhook_validation_test.go index 421caead..ddd3dced 100644 --- a/service/worker_slice_gateway_webhook_validation_test.go +++ b/service/worker_slice_gateway_webhook_validation_test.go @@ -18,9 +18,10 @@ package service import ( "context" - "k8s.io/apimachinery/pkg/runtime" "testing" + "k8s.io/apimachinery/pkg/runtime" + "github.com/dailymotion/allure-go" workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" "github.com/kubeslice/kubeslice-controller/util" @@ -43,6 +44,25 @@ func TestWorkerSliceGatewayWebhookValidationSuite(t *testing.T) { var WorkerSliceGatewayWebhookValidationTestBed = map[string]func(*testing.T){ "WorkerSliceGatewayWebhookValidation_UpdateValidateWorkerSliceGatewayUpdatingGatewayNumber": UpdateValidateWorkerSliceGatewayUpdatingGatewayNumber, "WorkerSliceGatewayWebhookValidation_UpdateValidateWorkerSliceGatewayWithoutErrors": UpdateValidateWorkerSliceGatewayWithoutErrors, + "WorkerSliceGatewayWebhookValidation_UpdateGatewayConnectivityType": UpdateGatewayConnectivityType, +} + +func UpdateGatewayConnectivityType(t *testing.T) { + name := "slice-cx-cy" + namespace := "stubns" + clientMock, newWorkerSliceGateway, ctx := setupWorkerSliceGatewayWebhookValidationTest(name, namespace) + existingWorkerSliceGateway := workerv1alpha1.WorkerSliceGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } + existingWorkerSliceGateway.Spec.GatewayConnectivityType = "LoadBalancer" + newWorkerSliceGateway.Spec.GatewayConnectivityType = "NodePort" + err := ValidateWorkerSliceGatewayUpdate(ctx, &existingWorkerSliceGateway, runtime.Object(newWorkerSliceGateway)) + require.NotNil(t, err) + require.Contains(t, err.Error(), "Spec.GatewayConnectivityType: Forbidden:") + clientMock.AssertExpectations(t) } func UpdateValidateWorkerSliceGatewayUpdatingGatewayNumber(t *testing.T) { From a7a4691890e355c29529df6f878a27075cada578 Mon Sep 17 00:00:00 2001 From: Mridul Date: Fri, 20 Oct 2023 15:01:02 +0530 Subject: [PATCH 06/13] set gwSvcType during workersliceconfig creation Signed-off-by: Mridul --- service/mocks/IVpnKeyRotationService.go | 2 +- service/mocks/IWorkerSliceConfigService.go | 20 +++---- service/slice_config_service.go | 14 ++++- service/slice_config_service_test.go | 13 ++--- service/worker_slice_config_service.go | 59 +++++++++++---------- service/worker_slice_config_service_test.go | 11 ++-- 6 files changed, 70 insertions(+), 49 deletions(-) diff --git a/service/mocks/IVpnKeyRotationService.go b/service/mocks/IVpnKeyRotationService.go index 17ce1d8d..c2179a92 100644 --- a/service/mocks/IVpnKeyRotationService.go +++ b/service/mocks/IVpnKeyRotationService.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.22.1. DO NOT EDIT. +// Code generated by mockery v2.28.1. DO NOT EDIT. package mocks diff --git a/service/mocks/IWorkerSliceConfigService.go b/service/mocks/IWorkerSliceConfigService.go index 6538dd25..a4a130f2 100644 --- a/service/mocks/IWorkerSliceConfigService.go +++ b/service/mocks/IWorkerSliceConfigService.go @@ -5,7 +5,9 @@ package mocks import ( context "context" + controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" mock "github.com/stretchr/testify/mock" + reconcile "sigs.k8s.io/controller-runtime/pkg/reconcile" v1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" @@ -32,25 +34,25 @@ func (_m *IWorkerSliceConfigService) ComputeClusterMap(clusterNames []string, wo return r0 } -// CreateMinimalWorkerSliceConfig provides a mock function with given fields: ctx, clusters, namespace, label, name, sliceSubnet, clusterCidr -func (_m *IWorkerSliceConfigService) CreateMinimalWorkerSliceConfig(ctx context.Context, clusters []string, namespace string, label map[string]string, name string, sliceSubnet string, clusterCidr string) (map[string]int, error) { - ret := _m.Called(ctx, clusters, namespace, label, name, sliceSubnet, clusterCidr) +// CreateMinimalWorkerSliceConfig provides a mock function with given fields: ctx, clusters, namespace, label, name, sliceSubnet, clusterCidr, sliceGwSvcTypeMap +func (_m *IWorkerSliceConfigService) CreateMinimalWorkerSliceConfig(ctx context.Context, clusters []string, namespace string, label map[string]string, name string, sliceSubnet string, clusterCidr string, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType) (map[string]int, error) { + ret := _m.Called(ctx, clusters, namespace, label, name, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) var r0 map[string]int var r1 error - if rf, ok := ret.Get(0).(func(context.Context, []string, string, map[string]string, string, string, string) (map[string]int, error)); ok { - return rf(ctx, clusters, namespace, label, name, sliceSubnet, clusterCidr) + if rf, ok := ret.Get(0).(func(context.Context, []string, string, map[string]string, string, string, string, map[string]*controllerv1alpha1.SliceGatewayServiceType) (map[string]int, error)); ok { + return rf(ctx, clusters, namespace, label, name, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) } - if rf, ok := ret.Get(0).(func(context.Context, []string, string, map[string]string, string, string, string) map[string]int); ok { - r0 = rf(ctx, clusters, namespace, label, name, sliceSubnet, clusterCidr) + if rf, ok := ret.Get(0).(func(context.Context, []string, string, map[string]string, string, string, string, map[string]*controllerv1alpha1.SliceGatewayServiceType) map[string]int); ok { + r0 = rf(ctx, clusters, namespace, label, name, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(map[string]int) } } - if rf, ok := ret.Get(1).(func(context.Context, []string, string, map[string]string, string, string, string) error); ok { - r1 = rf(ctx, clusters, namespace, label, name, sliceSubnet, clusterCidr) + if rf, ok := ret.Get(1).(func(context.Context, []string, string, map[string]string, string, string, string, map[string]*controllerv1alpha1.SliceGatewayServiceType) error); ok { + r1 = rf(ctx, clusters, namespace, label, name, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) } else { r1 = ret.Error(1) } diff --git a/service/slice_config_service.go b/service/slice_config_service.go index 9e9d9d4d..8d864867 100644 --- a/service/slice_config_service.go +++ b/service/slice_config_service.go @@ -19,6 +19,7 @@ package service import ( "context" "fmt" + "github.com/kubeslice/kubeslice-controller/metrics" "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" @@ -132,7 +133,18 @@ func (s *SliceConfigService) ReconcileSliceConfig(ctx context.Context, req ctrl. clusterCidr := util.FindCIDRByMaxClusters(sliceConfig.Spec.MaxClusters) completeResourceName := fmt.Sprintf(util.LabelValue, util.GetObjectKind(sliceConfig), sliceConfig.GetName()) ownershipLabel := util.GetOwnerLabel(completeResourceName) - clusterMap, err := s.ms.CreateMinimalWorkerSliceConfig(ctx, sliceConfig.Spec.Clusters, req.Namespace, ownershipLabel, sliceConfig.Name, sliceConfig.Spec.SliceSubnet, clusterCidr) + // create cluster wise slice gw svc info + var sliceGwSvcTypeMap = make(map[string]*v1alpha1.SliceGatewayServiceType) + for _, gwSvctype := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { + if gwSvctype.Cluster == "*" { + for _, cluster := range sliceConfig.Spec.Clusters { + sliceGwSvcTypeMap[cluster] = &gwSvctype + } + } else { + sliceGwSvcTypeMap[gwSvctype.Cluster] = &gwSvctype + } + } + clusterMap, err := s.ms.CreateMinimalWorkerSliceConfig(ctx, sliceConfig.Spec.Clusters, req.Namespace, ownershipLabel, sliceConfig.Name, sliceConfig.Spec.SliceSubnet, clusterCidr, sliceGwSvcTypeMap) if err != nil { return ctrl.Result{}, err } diff --git a/service/slice_config_service_test.go b/service/slice_config_service_test.go index a09d414b..3b143fa2 100644 --- a/service/slice_config_service_test.go +++ b/service/slice_config_service_test.go @@ -20,9 +20,10 @@ import ( "context" "errors" "fmt" + "testing" + "github.com/kubeslice/kubeslice-controller/metrics" metricMock "github.com/kubeslice/kubeslice-controller/metrics/mocks" - "testing" "github.com/kubeslice/kubeslice-monitoring/pkg/events" "k8s.io/apimachinery/pkg/runtime" @@ -99,7 +100,7 @@ func SliceConfigReconciliationCompleteHappyCase(t *testing.T) { "cluster-2": 2, } - workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() + workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything).Return(ctrl.Result{}, nil).Once() label := map[string]string{ "original-slice-name": sliceConfig.Name, @@ -279,7 +280,7 @@ func SliceConfigErrorOnCreateWorkerSliceConfig(t *testing.T) { "cluster-2": 2, } err1 := errors.New("internal_error") - workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, err1).Once() + workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, err1).Once() result, err2 := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} require.Error(t, err2) @@ -310,7 +311,7 @@ func SliceConfigErrorOnCreateWorkerSliceGateway(t *testing.T) { "cluster-1": 1, "cluster-2": 2, } - workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() + workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() err1 := errors.New("internal_error") workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything).Return(ctrl.Result{}, err1).Once() result, err2 := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) @@ -586,7 +587,7 @@ func SliceConfigErrorOnListingServiceExport(t *testing.T) { "cluster-1": 1, "cluster-2": 2, } - workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() + workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything).Return(ctrl.Result{}, nil).Once() label := map[string]string{ "original-slice-name": sliceConfig.Name, @@ -625,7 +626,7 @@ func SliceConfigErrorOnCreateOrUpdateServiceImport(t *testing.T) { "cluster-1": 1, "cluster-2": 2, } - workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() + workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything).Return(ctrl.Result{}, nil).Once() label := map[string]string{ "original-slice-name": sliceConfig.Name, diff --git a/service/worker_slice_config_service.go b/service/worker_slice_config_service.go index 94ce1a73..3d956186 100644 --- a/service/worker_slice_config_service.go +++ b/service/worker_slice_config_service.go @@ -21,20 +21,18 @@ import ( "fmt" "time" - "github.com/kubeslice/kubeslice-controller/metrics" - - "github.com/kubeslice/kubeslice-controller/events" - - "go.uber.org/zap" - "github.com/jinzhu/copier" - controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" - workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" - "github.com/kubeslice/kubeslice-controller/util" + "go.uber.org/zap" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + + controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" + workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" + "github.com/kubeslice/kubeslice-controller/events" + "github.com/kubeslice/kubeslice-controller/metrics" + "github.com/kubeslice/kubeslice-controller/util" ) const workerSliceConfigNameFormat = "%s-%s" @@ -44,7 +42,7 @@ type IWorkerSliceConfigService interface { DeleteWorkerSliceConfigByLabel(ctx context.Context, label map[string]string, namespace string) error ListWorkerSliceConfigs(ctx context.Context, ownerLabel map[string]string, namespace string) ([]workerv1alpha1.WorkerSliceConfig, error) ComputeClusterMap(clusterNames []string, workerSliceConfigs []workerv1alpha1.WorkerSliceConfig) map[string]int - CreateMinimalWorkerSliceConfig(ctx context.Context, clusters []string, namespace string, label map[string]string, name, sliceSubnet string, clusterCidr string) (map[string]int, error) + CreateMinimalWorkerSliceConfig(ctx context.Context, clusters []string, namespace string, label map[string]string, name, sliceSubnet string, clusterCidr string, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType) (map[string]int, error) } // WorkerSliceConfigService implements the IWorkerSliceConfigService interface @@ -214,21 +212,21 @@ outer: logger.With(zap.Error(err)).Errorf("Failed to deep copy external gateway configuration") } - // Reconcile Slice gateway service type - sliceGatewayProvider := workerv1alpha1.WorkerSliceGatewayProvider{ - SliceGatewayType: sliceConfig.Spec.SliceGatewayProvider.SliceGatewayType, - SliceCaType: sliceConfig.Spec.SliceGatewayProvider.SliceCaType, - } - gwSvcTypePresent := false - for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { - if gwSvcType.Cluster == "*" || gwSvcType.Cluster == workerSliceConfig.Labels["worker-cluster"] { - sliceGatewayProvider.SliceGatewayServiceType = gwSvcType.Type - gwSvcTypePresent = true - } - } - if !gwSvcTypePresent { - sliceGatewayProvider.SliceGatewayServiceType = defaultSliceGatewayServiceType - } + // // Reconcile Slice gateway service type + // sliceGatewayProvider := workerv1alpha1.WorkerSliceGatewayProvider{ + // SliceGatewayType: sliceConfig.Spec.SliceGatewayProvider.SliceGatewayType, + // SliceCaType: sliceConfig.Spec.SliceGatewayProvider.SliceCaType, + // } + // gwSvcTypePresent := false + // for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { + // if gwSvcType.Cluster == "*" || gwSvcType.Cluster == workerSliceConfig.Labels["worker-cluster"] { + // sliceGatewayProvider.SliceGatewayServiceType = gwSvcType.Type + // gwSvcTypePresent = true + // } + // } + // if !gwSvcTypePresent { + // sliceGatewayProvider.SliceGatewayServiceType = defaultSliceGatewayServiceType + // } // Reconcile the Namespace Isolation Profile controllerIsolationProfile := sliceConfig.Spec.NamespaceIsolationProfile @@ -256,7 +254,7 @@ outer: } workerSliceConfig.Spec.ExternalGatewayConfig = externalGatewayConfig - workerSliceConfig.Spec.SliceGatewayProvider = sliceGatewayProvider + // workerSliceConfig.Spec.SliceGatewayProvider = sliceGatewayProvider workerSliceConfig.Spec.NamespaceIsolationProfile = workerIsolationProfile workerSliceConfig.Spec.SliceName = sliceConfig.Name workerSliceConfig.Spec.Octet = octet @@ -270,7 +268,7 @@ outer: // CreateMinimalWorkerSliceConfig CreateWorkerSliceConfig is a function to create the worker slice configs with minimum number of fields. // More fields are added in reconciliation loop. -func (s *WorkerSliceConfigService) CreateMinimalWorkerSliceConfig(ctx context.Context, clusters []string, namespace string, label map[string]string, name, sliceSubnet string, clusterCidr string) (map[string]int, error) { +func (s *WorkerSliceConfigService) CreateMinimalWorkerSliceConfig(ctx context.Context, clusters []string, namespace string, label map[string]string, name, sliceSubnet string, clusterCidr string, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType) (map[string]int, error) { logger := util.CtxLogger(ctx) //Load Event Recorder with project name, slice name and namespace @@ -307,6 +305,11 @@ func (s *WorkerSliceConfigService) CreateMinimalWorkerSliceConfig(ctx context.Co } ipamOctet := clusterMap[cluster] clusterSubnetCIDR := fmt.Sprintf(util.GetClusterPrefixPool(sliceSubnet, ipamOctet, clusterCidr)) + // determine gw svc type + sliceGwSvcType := defaultSliceGatewayServiceType + if val, exists := sliceGwSvcTypeMap[cluster]; exists { + sliceGwSvcType = val.Type + } if !found { label["project-namespace"] = namespace label["original-slice-name"] = name @@ -325,6 +328,7 @@ func (s *WorkerSliceConfigService) CreateMinimalWorkerSliceConfig(ctx context.Co expectedSlice.Spec.Octet = &ipamOctet expectedSlice.Spec.ClusterSubnetCIDR = clusterSubnetCIDR expectedSlice.Spec.SliceSubnet = sliceSubnet + expectedSlice.Spec.SliceGatewayProvider.SliceGatewayServiceType = sliceGwSvcType err = util.CreateResource(ctx, &expectedSlice) if err != nil { //Register an event for worker slice config creation failure @@ -357,6 +361,7 @@ func (s *WorkerSliceConfigService) CreateMinimalWorkerSliceConfig(ctx context.Co existingSlice.UID = "" existingSlice.Spec.Octet = &ipamOctet existingSlice.Spec.ClusterSubnetCIDR = clusterSubnetCIDR + existingSlice.Spec.SliceGatewayProvider.SliceGatewayServiceType = sliceGwSvcType logger.Debug("updating slice with new octet", existingSlice) if existingSlice.Annotations == nil { existingSlice.Annotations = make(map[string]string) diff --git a/service/worker_slice_config_service_test.go b/service/worker_slice_config_service_test.go index abc77425..b3baa113 100644 --- a/service/worker_slice_config_service_test.go +++ b/service/worker_slice_config_service_test.go @@ -20,9 +20,10 @@ import ( "context" "errors" "fmt" + "testing" + "github.com/kubeslice/kubeslice-controller/metrics" metricMock "github.com/kubeslice/kubeslice-controller/metrics/mocks" - "testing" "github.com/kubeslice/kubeslice-monitoring/pkg/events" "k8s.io/apimachinery/pkg/runtime" @@ -338,7 +339,7 @@ func testCreateWorkerSliceConfigNewClusterSuccess(t *testing.T) { clientMock.On("Create", ctx, mock.Anything).Return(nil).Once() clientMock.On("Create", ctx, mock.AnythingOfType("*v1.Event")).Return(nil).Once() mMock.On("RecordCounterMetric", mock.Anything, mock.Anything).Return().Once() - result, err := WorkerSliceService.CreateMinimalWorkerSliceConfig(ctx, []string{"cluster-1", "cluster-2"}, requestObj.Namespace, label, "red", "198.23.54.47/16", "/20") + result, err := WorkerSliceService.CreateMinimalWorkerSliceConfig(ctx, []string{"cluster-1", "cluster-2"}, requestObj.Namespace, label, "red", "198.23.54.47/16", "/20", nil) require.Equal(t, len(result), 2) require.NoError(t, nil) require.Nil(t, err) @@ -405,7 +406,7 @@ func testCreateWorkerSliceConfigNewClusterFails(t *testing.T) { clientMock.On("Create", ctx, mock.Anything).Return(err1).Once() clientMock.On("Create", ctx, mock.AnythingOfType("*v1.Event")).Return(nil).Once() mMock.On("RecordCounterMetric", mock.Anything, mock.Anything).Return().Once() - result, err := WorkerSliceService.CreateMinimalWorkerSliceConfig(ctx, []string{"cluster-1", "cluster-2"}, requestObj.Namespace, label, "red", "198.23.54.47/16", "/20") + result, err := WorkerSliceService.CreateMinimalWorkerSliceConfig(ctx, []string{"cluster-1", "cluster-2"}, requestObj.Namespace, label, "red", "198.23.54.47/16", "/20", nil) require.Error(t, err) require.Equal(t, len(result), 2) require.Equal(t, err, err1) @@ -472,7 +473,7 @@ func testCreateWorkerSliceConfigUpdateClusterSuccess(t *testing.T) { mMock.On("RecordCounterMetric", mock.Anything, mock.Anything).Return().Once() clientMock.On("Update", ctx, mock.AnythingOfType("*v1.Event")).Return(nil).Once() mMock.On("RecordCounterMetric", mock.Anything, mock.Anything).Return().Once() - result, err := WorkerSliceService.CreateMinimalWorkerSliceConfig(ctx, []string{"cluster-1", "cluster-2"}, requestObj.Namespace, label, "red", "198.23.54.47/16", "/20") + result, err := WorkerSliceService.CreateMinimalWorkerSliceConfig(ctx, []string{"cluster-1", "cluster-2"}, requestObj.Namespace, label, "red", "198.23.54.47/16", "/20", nil) require.Equal(t, len(result), 2) require.NoError(t, nil) require.Nil(t, err) @@ -538,7 +539,7 @@ func testCreateWorkerSliceConfigUpdateClusterFails(t *testing.T) { clientMock.On("Update", ctx, mock.Anything).Return(err1).Once() clientMock.On("Create", ctx, mock.AnythingOfType("*v1.Event")).Return(nil).Once() mMock.On("RecordCounterMetric", mock.Anything, mock.Anything).Return().Once() - result, err := WorkerSliceService.CreateMinimalWorkerSliceConfig(ctx, []string{"cluster-1", "cluster-2"}, requestObj.Namespace, label, "red", "198.23.54.47/16", "/20") + result, err := WorkerSliceService.CreateMinimalWorkerSliceConfig(ctx, []string{"cluster-1", "cluster-2"}, requestObj.Namespace, label, "red", "198.23.54.47/16", "/20", nil) require.Error(t, err) require.Equal(t, len(result), 2) require.Equal(t, err, err1) From 42db3fcf6226145b672114744bb96fd78f51a036 Mon Sep 17 00:00:00 2001 From: Mridul Date: Fri, 20 Oct 2023 15:33:27 +0530 Subject: [PATCH 07/13] feat(AM-11526): set gwSvcType during workerSliceGw creation Signed-off-by: Mridul --- service/mocks/IWorkerSliceGatewayService.go | 42 +++++++------- service/slice_config_service.go | 4 +- service/slice_config_service_test.go | 8 +-- service/worker_slice_gateway_service.go | 59 +++++++++++--------- service/worker_slice_gateway_service_test.go | 7 ++- 5 files changed, 64 insertions(+), 56 deletions(-) diff --git a/service/mocks/IWorkerSliceGatewayService.go b/service/mocks/IWorkerSliceGatewayService.go index cbceba23..51a827c7 100644 --- a/service/mocks/IWorkerSliceGatewayService.go +++ b/service/mocks/IWorkerSliceGatewayService.go @@ -5,14 +5,14 @@ package mocks import ( context "context" - controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" mock "github.com/stretchr/testify/mock" - reconcile "sigs.k8s.io/controller-runtime/pkg/reconcile" util "github.com/kubeslice/kubeslice-controller/util" - v1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" + v1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" + + workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" ) // IWorkerSliceGatewayService is an autogenerated mock type for the IWorkerSliceGatewayService type @@ -34,23 +34,23 @@ func (_m *IWorkerSliceGatewayService) BuildNetworkAddresses(sliceSubnet string, return r0 } -// CreateMinimumWorkerSliceGateways provides a mock function with given fields: ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr -func (_m *IWorkerSliceGatewayService) CreateMinimumWorkerSliceGateways(ctx context.Context, sliceName string, clusterNames []string, namespace string, label map[string]string, clusterMap map[string]int, sliceSubnet string, clusterCidr string) (reconcile.Result, error) { - ret := _m.Called(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr) +// CreateMinimumWorkerSliceGateways provides a mock function with given fields: ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap +func (_m *IWorkerSliceGatewayService) CreateMinimumWorkerSliceGateways(ctx context.Context, sliceName string, clusterNames []string, namespace string, label map[string]string, clusterMap map[string]int, sliceSubnet string, clusterCidr string, sliceGwSvcTypeMap map[string]*v1alpha1.SliceGatewayServiceType) (reconcile.Result, error) { + ret := _m.Called(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) var r0 reconcile.Result var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, []string, string, map[string]string, map[string]int, string, string) (reconcile.Result, error)); ok { - return rf(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr) + if rf, ok := ret.Get(0).(func(context.Context, string, []string, string, map[string]string, map[string]int, string, string, map[string]*v1alpha1.SliceGatewayServiceType) (reconcile.Result, error)); ok { + return rf(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) } - if rf, ok := ret.Get(0).(func(context.Context, string, []string, string, map[string]string, map[string]int, string, string) reconcile.Result); ok { - r0 = rf(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr) + if rf, ok := ret.Get(0).(func(context.Context, string, []string, string, map[string]string, map[string]int, string, string, map[string]*v1alpha1.SliceGatewayServiceType) reconcile.Result); ok { + r0 = rf(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) } else { r0 = ret.Get(0).(reconcile.Result) } - if rf, ok := ret.Get(1).(func(context.Context, string, []string, string, map[string]string, map[string]int, string, string) error); ok { - r1 = rf(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr) + if rf, ok := ret.Get(1).(func(context.Context, string, []string, string, map[string]string, map[string]int, string, string, map[string]*v1alpha1.SliceGatewayServiceType) error); ok { + r1 = rf(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) } else { r1 = ret.Error(1) } @@ -73,11 +73,11 @@ func (_m *IWorkerSliceGatewayService) DeleteWorkerSliceGatewaysByLabel(ctx conte } // GenerateCerts provides a mock function with given fields: ctx, sliceName, namespace, serverGateway, clientGateway, gatewayAddresses -func (_m *IWorkerSliceGatewayService) GenerateCerts(ctx context.Context, sliceName string, namespace string, serverGateway *v1alpha1.WorkerSliceGateway, clientGateway *v1alpha1.WorkerSliceGateway, gatewayAddresses util.WorkerSliceGatewayNetworkAddresses) error { +func (_m *IWorkerSliceGatewayService) GenerateCerts(ctx context.Context, sliceName string, namespace string, serverGateway *workerv1alpha1.WorkerSliceGateway, clientGateway *workerv1alpha1.WorkerSliceGateway, gatewayAddresses util.WorkerSliceGatewayNetworkAddresses) error { ret := _m.Called(ctx, sliceName, namespace, serverGateway, clientGateway, gatewayAddresses) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, *v1alpha1.WorkerSliceGateway, *v1alpha1.WorkerSliceGateway, util.WorkerSliceGatewayNetworkAddresses) error); ok { + if rf, ok := ret.Get(0).(func(context.Context, string, string, *workerv1alpha1.WorkerSliceGateway, *workerv1alpha1.WorkerSliceGateway, util.WorkerSliceGatewayNetworkAddresses) error); ok { r0 = rf(ctx, sliceName, namespace, serverGateway, clientGateway, gatewayAddresses) } else { r0 = ret.Error(0) @@ -87,19 +87,19 @@ func (_m *IWorkerSliceGatewayService) GenerateCerts(ctx context.Context, sliceNa } // ListWorkerSliceGateways provides a mock function with given fields: ctx, ownerLabel, namespace -func (_m *IWorkerSliceGatewayService) ListWorkerSliceGateways(ctx context.Context, ownerLabel map[string]string, namespace string) ([]v1alpha1.WorkerSliceGateway, error) { +func (_m *IWorkerSliceGatewayService) ListWorkerSliceGateways(ctx context.Context, ownerLabel map[string]string, namespace string) ([]workerv1alpha1.WorkerSliceGateway, error) { ret := _m.Called(ctx, ownerLabel, namespace) - var r0 []v1alpha1.WorkerSliceGateway + var r0 []workerv1alpha1.WorkerSliceGateway var r1 error - if rf, ok := ret.Get(0).(func(context.Context, map[string]string, string) ([]v1alpha1.WorkerSliceGateway, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, map[string]string, string) ([]workerv1alpha1.WorkerSliceGateway, error)); ok { return rf(ctx, ownerLabel, namespace) } - if rf, ok := ret.Get(0).(func(context.Context, map[string]string, string) []v1alpha1.WorkerSliceGateway); ok { + if rf, ok := ret.Get(0).(func(context.Context, map[string]string, string) []workerv1alpha1.WorkerSliceGateway); ok { r0 = rf(ctx, ownerLabel, namespace) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]v1alpha1.WorkerSliceGateway) + r0 = ret.Get(0).([]workerv1alpha1.WorkerSliceGateway) } } @@ -113,11 +113,11 @@ func (_m *IWorkerSliceGatewayService) ListWorkerSliceGateways(ctx context.Contex } // NodeIpReconciliationOfWorkerSliceGateways provides a mock function with given fields: ctx, cluster, namespace -func (_m *IWorkerSliceGatewayService) NodeIpReconciliationOfWorkerSliceGateways(ctx context.Context, cluster *controllerv1alpha1.Cluster, namespace string) error { +func (_m *IWorkerSliceGatewayService) NodeIpReconciliationOfWorkerSliceGateways(ctx context.Context, cluster *v1alpha1.Cluster, namespace string) error { ret := _m.Called(ctx, cluster, namespace) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, *controllerv1alpha1.Cluster, string) error); ok { + if rf, ok := ret.Get(0).(func(context.Context, *v1alpha1.Cluster, string) error); ok { r0 = rf(ctx, cluster, namespace) } else { r0 = ret.Error(0) diff --git a/service/slice_config_service.go b/service/slice_config_service.go index 8d864867..730ddd1f 100644 --- a/service/slice_config_service.go +++ b/service/slice_config_service.go @@ -133,7 +133,7 @@ func (s *SliceConfigService) ReconcileSliceConfig(ctx context.Context, req ctrl. clusterCidr := util.FindCIDRByMaxClusters(sliceConfig.Spec.MaxClusters) completeResourceName := fmt.Sprintf(util.LabelValue, util.GetObjectKind(sliceConfig), sliceConfig.GetName()) ownershipLabel := util.GetOwnerLabel(completeResourceName) - // create cluster wise slice gw svc info + // collect cluster wise slice gw svc info var sliceGwSvcTypeMap = make(map[string]*v1alpha1.SliceGatewayServiceType) for _, gwSvctype := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { if gwSvctype.Cluster == "*" { @@ -150,7 +150,7 @@ func (s *SliceConfigService) ReconcileSliceConfig(ctx context.Context, req ctrl. } // Step 4: Create gateways with minimum specification - _, err = s.sgs.CreateMinimumWorkerSliceGateways(ctx, sliceConfig.Name, sliceConfig.Spec.Clusters, req.Namespace, ownershipLabel, clusterMap, sliceConfig.Spec.SliceSubnet, clusterCidr) + _, err = s.sgs.CreateMinimumWorkerSliceGateways(ctx, sliceConfig.Name, sliceConfig.Spec.Clusters, req.Namespace, ownershipLabel, clusterMap, sliceConfig.Spec.SliceSubnet, clusterCidr, sliceGwSvcTypeMap) if err != nil { return ctrl.Result{}, err } diff --git a/service/slice_config_service_test.go b/service/slice_config_service_test.go index 3b143fa2..14279003 100644 --- a/service/slice_config_service_test.go +++ b/service/slice_config_service_test.go @@ -101,7 +101,7 @@ func SliceConfigReconciliationCompleteHappyCase(t *testing.T) { } workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() - workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything).Return(ctrl.Result{}, nil).Once() + workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything, mock.Anything).Return(ctrl.Result{}, nil).Once() label := map[string]string{ "original-slice-name": sliceConfig.Name, } @@ -313,7 +313,7 @@ func SliceConfigErrorOnCreateWorkerSliceGateway(t *testing.T) { } workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() err1 := errors.New("internal_error") - workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything).Return(ctrl.Result{}, err1).Once() + workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything, mock.Anything).Return(ctrl.Result{}, err1).Once() result, err2 := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} require.Error(t, err2) @@ -588,7 +588,7 @@ func SliceConfigErrorOnListingServiceExport(t *testing.T) { "cluster-2": 2, } workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() - workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything).Return(ctrl.Result{}, nil).Once() + workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything, mock.Anything).Return(ctrl.Result{}, nil).Once() label := map[string]string{ "original-slice-name": sliceConfig.Name, } @@ -627,7 +627,7 @@ func SliceConfigErrorOnCreateOrUpdateServiceImport(t *testing.T) { "cluster-2": 2, } workerSliceConfigMock.On("CreateMinimalWorkerSliceConfig", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(clusterMap, nil).Once() - workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything).Return(ctrl.Result{}, nil).Once() + workerSliceGatewayMock.On("CreateMinimumWorkerSliceGateways", ctx, mock.Anything, mock.Anything, requestObj.Namespace, mock.Anything, clusterMap, mock.Anything, mock.Anything, mock.Anything).Return(ctrl.Result{}, nil).Once() label := map[string]string{ "original-slice-name": sliceConfig.Name, } diff --git a/service/worker_slice_gateway_service.go b/service/worker_slice_gateway_service.go index 66e60171..3a3d1b04 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -46,7 +46,7 @@ const gatewayName = "%s-%s-%s" type IWorkerSliceGatewayService interface { ReconcileWorkerSliceGateways(ctx context.Context, req ctrl.Request) (ctrl.Result, error) CreateMinimumWorkerSliceGateways(ctx context.Context, sliceName string, clusterNames []string, namespace string, - label map[string]string, clusterMap map[string]int, sliceSubnet string, clusterCidr string) (ctrl.Result, error) + label map[string]string, clusterMap map[string]int, sliceSubnet string, clusterCidr string, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType) (ctrl.Result, error) ListWorkerSliceGateways(ctx context.Context, ownerLabel map[string]string, namespace string) ([]v1alpha1.WorkerSliceGateway, error) DeleteWorkerSliceGatewaysByLabel(ctx context.Context, label map[string]string, namespace string) error NodeIpReconciliationOfWorkerSliceGateways(ctx context.Context, cluster *controllerv1alpha1.Cluster, namespace string) error @@ -207,20 +207,20 @@ func (s *WorkerSliceGatewayService) ReconcileWorkerSliceGateways(ctx context.Con logger.Infof("sliceConfig %v not found, returning from reconciler loop.", req.NamespacedName) return ctrl.Result{}, nil } - // reconcile gateway connectivity type - var clusterName string - if workerSliceGateway.Spec.GatewayHostType == serverGateway { - clusterName = workerSliceGateway.Labels["worker-cluster"] - } else { - clusterName = workerSliceGateway.Labels["remote-cluster"] - } - gatewayConnectivityType := defaultSliceGatewayServiceType - for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { - if gwSvcType.Cluster == "*" || gwSvcType.Cluster == clusterName { - gatewayConnectivityType = gwSvcType.Type - } - } - workerSliceGateway.Spec.GatewayConnectivityType = gatewayConnectivityType + // // reconcile gateway connectivity type + // var clusterName string + // if workerSliceGateway.Spec.GatewayHostType == serverGateway { + // clusterName = workerSliceGateway.Labels["worker-cluster"] + // } else { + // clusterName = workerSliceGateway.Labels["remote-cluster"] + // } + // gatewayConnectivityType := defaultSliceGatewayServiceType + // for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { + // if gwSvcType.Cluster == "*" || gwSvcType.Cluster == clusterName { + // gatewayConnectivityType = gwSvcType.Type + // } + // } + // workerSliceGateway.Spec.GatewayConnectivityType = gatewayConnectivityType logger.Debugf("setting gwsvctype %s", workerSliceGateway.Spec.GatewayConnectivityType) workerSliceGateway.Spec.GatewayType = workerSliceGatewayType workerSliceGateway.UID = "" @@ -343,7 +343,7 @@ type IndividualCertPairRequest struct { // CreateMinimumWorkerSliceGateways is a function to create gateways with minimum specification func (s *WorkerSliceGatewayService) CreateMinimumWorkerSliceGateways(ctx context.Context, sliceName string, clusterNames []string, namespace string, label map[string]string, clusterMap map[string]int, - sliceSubnet string, clusterCidr string) (ctrl.Result, error) { + sliceSubnet string, clusterCidr string, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType) (ctrl.Result, error) { err := s.cleanupObsoleteGateways(ctx, namespace, label, clusterNames, clusterMap) if err != nil { @@ -353,7 +353,7 @@ func (s *WorkerSliceGatewayService) CreateMinimumWorkerSliceGateways(ctx context return ctrl.Result{}, nil } - _, err = s.createMinimumGatewaysIfNotExists(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr) + _, err = s.createMinimumGatewaysIfNotExists(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) if err != nil { return ctrl.Result{}, err } @@ -433,7 +433,7 @@ func (s *WorkerSliceGatewayService) cleanupObsoleteGateways(ctx context.Context, // createMinimumGatewaysIfNotExists is a helper function to create the gateways between worker clusters if not exists func (s *WorkerSliceGatewayService) createMinimumGatewaysIfNotExists(ctx context.Context, sliceName string, clusterNames []string, namespace string, ownerLabel map[string]string, clusterMap map[string]int, - sliceSubnet string, clusterCidr string) (ctrl.Result, error) { + sliceSubnet string, clusterCidr string, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType) (ctrl.Result, error) { noClusters := len(clusterNames) clusterMapping := map[string]*controllerv1alpha1.Cluster{} for _, clusterName := range clusterNames { @@ -449,7 +449,12 @@ func (s *WorkerSliceGatewayService) createMinimumGatewaysIfNotExists(ctx context sourceCluster, destinationCluster := clusterMapping[clusterNames[i]], clusterMapping[clusterNames[j]] gatewayNumber := s.calculateGatewayNumber(clusterMap[sourceCluster.Name], clusterMap[destinationCluster.Name]) gatewayAddresses := s.BuildNetworkAddresses(sliceSubnet, sourceCluster.Name, destinationCluster.Name, clusterMap, clusterCidr) - err := s.createMinimumGateWayPairIfNotExists(ctx, sourceCluster, destinationCluster, sliceName, namespace, ownerLabel, gatewayNumber, gatewayAddresses) + // determine the gateway svc parameters + sliceGwSvcType := defaultSliceGatewayServiceType + if val, exists := sliceGwSvcTypeMap[sourceCluster.Name]; exists { + sliceGwSvcType = val.Type + } + err := s.createMinimumGateWayPairIfNotExists(ctx, sourceCluster, destinationCluster, sliceName, namespace, sliceGwSvcType, ownerLabel, gatewayNumber, gatewayAddresses) if err != nil { return ctrl.Result{}, err } @@ -461,8 +466,9 @@ func (s *WorkerSliceGatewayService) createMinimumGatewaysIfNotExists(ctx context // createMinimumGateWayPairIfNotExists is a function to create the pair of gatways between 2 clusters if not exists func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx context.Context, - sourceCluster *controllerv1alpha1.Cluster, destinationCluster *controllerv1alpha1.Cluster, sliceName string, namespace string, - label map[string]string, gatewayNumber int, gatewayAddresses util.WorkerSliceGatewayNetworkAddresses) error { + sourceCluster *controllerv1alpha1.Cluster, destinationCluster *controllerv1alpha1.Cluster, + sliceName, namespace, sliceGwSvcType string, label map[string]string, gatewayNumber int, + gatewayAddresses util.WorkerSliceGatewayNetworkAddresses) error { serverGatewayName := fmt.Sprintf(gatewayName, sliceName, sourceCluster.Name, destinationCluster.Name) clientGatewayName := fmt.Sprintf(gatewayName, sliceName, destinationCluster.Name, sourceCluster.Name) gateway := v1alpha1.WorkerSliceGateway{} @@ -493,7 +499,7 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont WithNamespace(namespace). WithSlice(sliceName) - serverGatewayObject := s.buildMinimumGateway(sourceCluster, destinationCluster, sliceName, namespace, label, serverGateway, gatewayNumber, gatewayAddresses.ServerSubnet, gatewayAddresses.ServerVpnAddress, clientGatewayName, gatewayAddresses.ClientSubnet, gatewayAddresses.ClientVpnAddress, serverGatewayName) + serverGatewayObject := s.buildMinimumGateway(sourceCluster, destinationCluster, sliceName, namespace, serverGateway, sliceGwSvcType, label, gatewayNumber, gatewayAddresses.ServerSubnet, gatewayAddresses.ServerVpnAddress, clientGatewayName, gatewayAddresses.ClientSubnet, gatewayAddresses.ClientVpnAddress, serverGatewayName) err = util.CreateResource(ctx, serverGatewayObject) if err != nil { //Register an event for worker slice gateway creation failure @@ -518,7 +524,7 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont "object_kind": metricKindWorkerSliceGateway, }, ) - clientGatewayObject := s.buildMinimumGateway(destinationCluster, sourceCluster, sliceName, namespace, label, clientGateway, gatewayNumber, gatewayAddresses.ClientSubnet, gatewayAddresses.ClientVpnAddress, serverGatewayName, gatewayAddresses.ServerSubnet, gatewayAddresses.ServerVpnAddress, clientGatewayName) + clientGatewayObject := s.buildMinimumGateway(destinationCluster, sourceCluster, sliceName, namespace, clientGateway, sliceGwSvcType, label, gatewayNumber, gatewayAddresses.ClientSubnet, gatewayAddresses.ClientVpnAddress, serverGatewayName, gatewayAddresses.ServerSubnet, gatewayAddresses.ServerVpnAddress, clientGatewayName) err = util.CreateResource(ctx, clientGatewayObject) if err != nil { //Register an event for worker slice gateway creation failure @@ -571,7 +577,7 @@ func (s *WorkerSliceGatewayService) BuildNetworkAddresses(sliceSubnet, sourceClu // buildMinimumGateway function returns the gateway object func (s *WorkerSliceGatewayService) buildMinimumGateway(sourceCluster, destinationCluster *controllerv1alpha1.Cluster, - sliceName, namespace string, labels map[string]string, gatewayHostType string, gatewayNumber int, + sliceName, namespace, gatewayHostType, sliceGwSvcType string, labels map[string]string, gatewayNumber int, gatewaySubnet, localVpnAddress, remoteGatewayName, remoteGatewaySubnet, remoteVpnAddress, localGatewayName string) *v1alpha1.WorkerSliceGateway { labels["worker-cluster"] = sourceCluster.Name labels["remote-cluster"] = destinationCluster.Name @@ -616,8 +622,9 @@ func (s *WorkerSliceGatewayService) buildMinimumGateway(sourceCluster, destinati GatewayCredentials: v1alpha1.GatewayCredentials{ SecretName: fmt.Sprintf(gatewayName, sliceName, sourceCluster.Name, destinationCluster.Name), }, - GatewayHostType: gatewayHostType, - GatewayNumber: gatewayNumber, + GatewayHostType: gatewayHostType, + GatewayNumber: gatewayNumber, + GatewayConnectivityType: sliceGwSvcType, }, } } diff --git a/service/worker_slice_gateway_service_test.go b/service/worker_slice_gateway_service_test.go index 563d4175..f09906b8 100644 --- a/service/worker_slice_gateway_service_test.go +++ b/service/worker_slice_gateway_service_test.go @@ -18,9 +18,10 @@ package service import ( "context" + "testing" + "github.com/kubeslice/kubeslice-controller/metrics" metricMock "github.com/kubeslice/kubeslice-controller/metrics/mocks" - "testing" "github.com/kubeslice/kubeslice-monitoring/pkg/events" "k8s.io/apimachinery/pkg/runtime" @@ -319,7 +320,7 @@ func testCreateMinimumWorkerSliceGatewaysAlreadyExists(t *testing.T) { //environment := make(map[string]string, 5) //jobMock.On("CreateJob", ctx, requestObj.Namespace, "image", environment).Return(ctrl.Result{}, nil).Once() - result, err := workerSliceGatewayService.CreateMinimumWorkerSliceGateways(ctx, "red", clusterNames, requestObj.Namespace, label, clusterMap, "10.10.10.10/16", "/16") + result, err := workerSliceGatewayService.CreateMinimumWorkerSliceGateways(ctx, "red", clusterNames, requestObj.Namespace, label, clusterMap, "10.10.10.10/16", "/16", nil) expectedResult := ctrl.Result{} require.NoError(t, nil) require.Equal(t, result, expectedResult) @@ -406,7 +407,7 @@ func testCreateMinimumWorkerSliceGatewaysNotExists(t *testing.T) { clientMock.On("Update", ctx, mock.AnythingOfType("*v1.Event")).Return(nil).Once() clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(nil).Once() mMock.On("RecordCounterMetric", mock.Anything, mock.Anything).Return().Once() - result, err := workerSliceGatewayService.CreateMinimumWorkerSliceGateways(ctx, "red", clusterNames, requestObj.Namespace, label, clusterMap, "10.10.10.10/16", "/16") + result, err := workerSliceGatewayService.CreateMinimumWorkerSliceGateways(ctx, "red", clusterNames, requestObj.Namespace, label, clusterMap, "10.10.10.10/16", "/16", nil) expectedResult := ctrl.Result{} require.NoError(t, nil) require.Equal(t, result, expectedResult) From b9879dfb68d167484dbcb54e042e1e9d15714279 Mon Sep 17 00:00:00 2001 From: Mridul Date: Wed, 25 Oct 2023 19:14:03 +0530 Subject: [PATCH 08/13] feat(AM-11526): webhook validation not needed in wsc & wsg Signed-off-by: Mridul --- service/worker_slice_config_service.go | 32 +++++++++---------- .../worker_slice_config_webhook_validation.go | 3 -- ...er_slice_config_webhook_validation_test.go | 19 ----------- service/worker_slice_gateway_service.go | 28 ++++++++-------- ...worker_slice_gateway_webhook_validation.go | 3 -- ...r_slice_gateway_webhook_validation_test.go | 19 ----------- 6 files changed, 30 insertions(+), 74 deletions(-) diff --git a/service/worker_slice_config_service.go b/service/worker_slice_config_service.go index 3d956186..4676faca 100644 --- a/service/worker_slice_config_service.go +++ b/service/worker_slice_config_service.go @@ -212,21 +212,21 @@ outer: logger.With(zap.Error(err)).Errorf("Failed to deep copy external gateway configuration") } - // // Reconcile Slice gateway service type - // sliceGatewayProvider := workerv1alpha1.WorkerSliceGatewayProvider{ - // SliceGatewayType: sliceConfig.Spec.SliceGatewayProvider.SliceGatewayType, - // SliceCaType: sliceConfig.Spec.SliceGatewayProvider.SliceCaType, - // } - // gwSvcTypePresent := false - // for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { - // if gwSvcType.Cluster == "*" || gwSvcType.Cluster == workerSliceConfig.Labels["worker-cluster"] { - // sliceGatewayProvider.SliceGatewayServiceType = gwSvcType.Type - // gwSvcTypePresent = true - // } - // } - // if !gwSvcTypePresent { - // sliceGatewayProvider.SliceGatewayServiceType = defaultSliceGatewayServiceType - // } + // Reconcile Slice gateway service type + sliceGatewayProvider := workerv1alpha1.WorkerSliceGatewayProvider{ + SliceGatewayType: sliceConfig.Spec.SliceGatewayProvider.SliceGatewayType, + SliceCaType: sliceConfig.Spec.SliceGatewayProvider.SliceCaType, + } + gwSvcTypePresent := false + for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { + if gwSvcType.Cluster == "*" || gwSvcType.Cluster == workerSliceConfig.Labels["worker-cluster"] { + sliceGatewayProvider.SliceGatewayServiceType = gwSvcType.Type + gwSvcTypePresent = true + } + } + if !gwSvcTypePresent { + sliceGatewayProvider.SliceGatewayServiceType = defaultSliceGatewayServiceType + } // Reconcile the Namespace Isolation Profile controllerIsolationProfile := sliceConfig.Spec.NamespaceIsolationProfile @@ -254,7 +254,7 @@ outer: } workerSliceConfig.Spec.ExternalGatewayConfig = externalGatewayConfig - // workerSliceConfig.Spec.SliceGatewayProvider = sliceGatewayProvider + workerSliceConfig.Spec.SliceGatewayProvider = sliceGatewayProvider workerSliceConfig.Spec.NamespaceIsolationProfile = workerIsolationProfile workerSliceConfig.Spec.SliceName = sliceConfig.Name workerSliceConfig.Spec.Octet = octet diff --git a/service/worker_slice_config_webhook_validation.go b/service/worker_slice_config_webhook_validation.go index e2359e81..2d8af128 100644 --- a/service/worker_slice_config_webhook_validation.go +++ b/service/worker_slice_config_webhook_validation.go @@ -41,8 +41,5 @@ func preventUpdateWorkerSliceConfig(ctx context.Context, ss *workerv1alpha1.Work if workerSliceConfig.Spec.Octet != nil && *workerSliceConfig.Spec.Octet != *ss.Spec.Octet { return field.Invalid(field.NewPath("Spec").Child("Octet"), *ss.Spec.Octet, "cannot be updated") } - if workerSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType != ss.Spec.SliceGatewayProvider.SliceGatewayType { - return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed") - } return nil } diff --git a/service/worker_slice_config_webhook_validation_test.go b/service/worker_slice_config_webhook_validation_test.go index ea899835..267d2c35 100644 --- a/service/worker_slice_config_webhook_validation_test.go +++ b/service/worker_slice_config_webhook_validation_test.go @@ -44,25 +44,6 @@ func TestWorkerSliceConfigWebhookValidationSuite(t *testing.T) { var WorkerSliceConfigWebhookValidationTestBed = map[string]func(*testing.T){ "WorkerSliceConfigWebhookValidation_UpdateValidateWorkerSliceConfigUpdatingOctet": UpdateValidateWorkerSliceConfigUpdatingOctet, "WorkerSliceConfigWebhookValidation_UpdateValidateWorkerSliceConfigWithoutErrors": UpdateValidateWorkerSliceConfigWithoutErrors, - "WorkerSliceConfigWebhookValidation_UpdateSliceGatewayServiceType": UpdateSliceGatewayServiceType, -} - -func UpdateSliceGatewayServiceType(t *testing.T) { - name := "slice-clusterx" - namespace := "demons" - clientMock, newWorkerSliceConfig, ctx := setupWorkerSliceConfigWebhookValidationTest(name, namespace) - existingWorkerSliceConfig := workerv1alpha1.WorkerSliceConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - }, - } - existingWorkerSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = "LoadBalancer" - newWorkerSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = "NodePort" - err := ValidateWorkerSliceConfigUpdate(ctx, &existingWorkerSliceConfig, runtime.Object(newWorkerSliceConfig)) - require.NotNil(t, err) - require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") - clientMock.AssertExpectations(t) } func UpdateValidateWorkerSliceConfigUpdatingOctet(t *testing.T) { diff --git a/service/worker_slice_gateway_service.go b/service/worker_slice_gateway_service.go index 3a3d1b04..bb6f06a0 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -207,20 +207,20 @@ func (s *WorkerSliceGatewayService) ReconcileWorkerSliceGateways(ctx context.Con logger.Infof("sliceConfig %v not found, returning from reconciler loop.", req.NamespacedName) return ctrl.Result{}, nil } - // // reconcile gateway connectivity type - // var clusterName string - // if workerSliceGateway.Spec.GatewayHostType == serverGateway { - // clusterName = workerSliceGateway.Labels["worker-cluster"] - // } else { - // clusterName = workerSliceGateway.Labels["remote-cluster"] - // } - // gatewayConnectivityType := defaultSliceGatewayServiceType - // for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { - // if gwSvcType.Cluster == "*" || gwSvcType.Cluster == clusterName { - // gatewayConnectivityType = gwSvcType.Type - // } - // } - // workerSliceGateway.Spec.GatewayConnectivityType = gatewayConnectivityType + // reconcile gateway connectivity type + var clusterName string + if workerSliceGateway.Spec.GatewayHostType == serverGateway { + clusterName = workerSliceGateway.Labels["worker-cluster"] + } else { + clusterName = workerSliceGateway.Labels["remote-cluster"] + } + gatewayConnectivityType := defaultSliceGatewayServiceType + for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { + if gwSvcType.Cluster == "*" || gwSvcType.Cluster == clusterName { + gatewayConnectivityType = gwSvcType.Type + } + } + workerSliceGateway.Spec.GatewayConnectivityType = gatewayConnectivityType logger.Debugf("setting gwsvctype %s", workerSliceGateway.Spec.GatewayConnectivityType) workerSliceGateway.Spec.GatewayType = workerSliceGatewayType workerSliceGateway.UID = "" diff --git a/service/worker_slice_gateway_webhook_validation.go b/service/worker_slice_gateway_webhook_validation.go index 9685159e..62e4eb2b 100644 --- a/service/worker_slice_gateway_webhook_validation.go +++ b/service/worker_slice_gateway_webhook_validation.go @@ -41,8 +41,5 @@ func preventUpdateWorkerSliceGateway(workerSliceGatewayCtx context.Context, sg * if workerSliceGateway.Spec.GatewayNumber != sg.Spec.GatewayNumber { return field.Invalid(field.NewPath("Spec").Child("GatewayNumber"), sg.Spec.GatewayNumber, "cannot be updated") } - if workerSliceGateway.Spec.GatewayConnectivityType != sg.Spec.GatewayConnectivityType { - return field.Forbidden(field.NewPath("Spec").Child("GatewayConnectivityType"), "update not allowed") - } return nil } diff --git a/service/worker_slice_gateway_webhook_validation_test.go b/service/worker_slice_gateway_webhook_validation_test.go index ddd3dced..406f9d23 100644 --- a/service/worker_slice_gateway_webhook_validation_test.go +++ b/service/worker_slice_gateway_webhook_validation_test.go @@ -44,25 +44,6 @@ func TestWorkerSliceGatewayWebhookValidationSuite(t *testing.T) { var WorkerSliceGatewayWebhookValidationTestBed = map[string]func(*testing.T){ "WorkerSliceGatewayWebhookValidation_UpdateValidateWorkerSliceGatewayUpdatingGatewayNumber": UpdateValidateWorkerSliceGatewayUpdatingGatewayNumber, "WorkerSliceGatewayWebhookValidation_UpdateValidateWorkerSliceGatewayWithoutErrors": UpdateValidateWorkerSliceGatewayWithoutErrors, - "WorkerSliceGatewayWebhookValidation_UpdateGatewayConnectivityType": UpdateGatewayConnectivityType, -} - -func UpdateGatewayConnectivityType(t *testing.T) { - name := "slice-cx-cy" - namespace := "stubns" - clientMock, newWorkerSliceGateway, ctx := setupWorkerSliceGatewayWebhookValidationTest(name, namespace) - existingWorkerSliceGateway := workerv1alpha1.WorkerSliceGateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - }, - } - existingWorkerSliceGateway.Spec.GatewayConnectivityType = "LoadBalancer" - newWorkerSliceGateway.Spec.GatewayConnectivityType = "NodePort" - err := ValidateWorkerSliceGatewayUpdate(ctx, &existingWorkerSliceGateway, runtime.Object(newWorkerSliceGateway)) - require.NotNil(t, err) - require.Contains(t, err.Error(), "Spec.GatewayConnectivityType: Forbidden:") - clientMock.AssertExpectations(t) } func UpdateValidateWorkerSliceGatewayUpdatingGatewayNumber(t *testing.T) { From bd3b57cdbfc22c66d36cc35001b5a66d51c92b1e Mon Sep 17 00:00:00 2001 From: Mridul Gain Date: Fri, 3 Nov 2023 11:55:54 +0530 Subject: [PATCH 09/13] feat(AM-11741): protocol specification for slice gateway load balancer (#190) * feat(AM-11741): define protocol for gw svc type * feat(AM-11741): reconcilation logic for gateway service protocol * feat(AM-11741): provide gw protocol during cert generation * fix(AM-11741): webhook validation, env var & wsc reconciler * fix(): gwsvctype mistatch --------- Signed-off-by: Mridul Gain --- apis/controller/v1alpha1/sliceconfig_types.go | 4 ++ .../v1alpha1/workersliceconfig_types.go | 3 ++ .../v1alpha1/workerslicegateway_types.go | 13 +++-- .../controller.kubeslice.io_sliceconfigs.yaml | 7 +++ ...orker.kubeslice.io_workersliceconfigs.yaml | 6 +++ ...rker.kubeslice.io_workerslicegateways.yaml | 6 +++ service/kube_slice_resource_names.go | 9 ++-- service/mocks/IWorkerSliceGatewayService.go | 10 ++-- service/service_helper.go | 17 +++++++ service/slice_config_service.go | 14 ++---- service/slice_config_webhook_validation.go | 2 +- .../slice_config_webhook_validation_test.go | 11 ++++ service/vpn_key_rotation_service.go | 2 +- service/vpn_key_rotation_service_test.go | 4 +- service/worker_slice_config_service.go | 35 +++++++------ service/worker_slice_gateway_service.go | 50 +++++++++++++------ 16 files changed, 133 insertions(+), 60 deletions(-) diff --git a/apis/controller/v1alpha1/sliceconfig_types.go b/apis/controller/v1alpha1/sliceconfig_types.go index 1a80d105..ce16514d 100644 --- a/apis/controller/v1alpha1/sliceconfig_types.go +++ b/apis/controller/v1alpha1/sliceconfig_types.go @@ -85,6 +85,10 @@ type SliceGatewayServiceType struct { //+kubebuilder:default:=NodePort //+kubebuilder:validation:Enum:=NodePort;LoadBalancer Type string `json:"type"` + // +kubebuilder:validation:Required + //+kubebuilder:default:=UDP + //+kubebuilder:validation:Enum:=TCP;UDP + Protocol string `json:"protocol"` } // QOSProfile is the QOS Profile configuration from backend diff --git a/apis/worker/v1alpha1/workersliceconfig_types.go b/apis/worker/v1alpha1/workersliceconfig_types.go index 55125e1f..3215b929 100644 --- a/apis/worker/v1alpha1/workersliceconfig_types.go +++ b/apis/worker/v1alpha1/workersliceconfig_types.go @@ -62,6 +62,9 @@ type WorkerSliceGatewayProvider struct { //+kubebuilder:default:=NodePort //+kubebuilder:validation:Enum:=NodePort;LoadBalancer SliceGatewayServiceType string `json:"sliceGatewayServiceType,omitempty"` + //+kubebuilder:default:=UDP + //+kubebuilder:validation:Enum:=TCP;UDP + SliceGatewayProtocol string `json:"sliceGatewayProtocol,omitempty"` } // QOSProfile is the QOS Profile configuration from backend diff --git a/apis/worker/v1alpha1/workerslicegateway_types.go b/apis/worker/v1alpha1/workerslicegateway_types.go index 82820e62..51e5fa03 100644 --- a/apis/worker/v1alpha1/workerslicegateway_types.go +++ b/apis/worker/v1alpha1/workerslicegateway_types.go @@ -32,11 +32,14 @@ type WorkerSliceGatewaySpec struct { GatewayHostType string `json:"gatewayHostType,omitempty"` //+kubebuilder:default:=NodePort //+kubebuilder:validation:Enum:=NodePort;LoadBalancer - GatewayConnectivityType string `json:"gatewayConnectivityType,omitempty"` - GatewayCredentials GatewayCredentials `json:"gatewayCredentials,omitempty"` - LocalGatewayConfig SliceGatewayConfig `json:"localGatewayConfig,omitempty"` - RemoteGatewayConfig SliceGatewayConfig `json:"remoteGatewayConfig,omitempty"` - GatewayNumber int `json:"gatewayNumber,omitempty"` + GatewayConnectivityType string `json:"gatewayConnectivityType,omitempty"` + //+kubebuilder:default:=UDP + //+kubebuilder:validation:Enum:=TCP;UDP + GatewayProtocol string `json:"gatewayProtocol,omitempty"` + GatewayCredentials GatewayCredentials `json:"gatewayCredentials,omitempty"` + LocalGatewayConfig SliceGatewayConfig `json:"localGatewayConfig,omitempty"` + RemoteGatewayConfig SliceGatewayConfig `json:"remoteGatewayConfig,omitempty"` + GatewayNumber int `json:"gatewayNumber,omitempty"` } type SliceGatewayConfig struct { diff --git a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml index 39adb202..35fa8ac9 100644 --- a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml +++ b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml @@ -164,6 +164,12 @@ spec: properties: cluster: type: string + protocol: + default: UDP + enum: + - TCP + - UDP + type: string type: default: NodePort enum: @@ -172,6 +178,7 @@ spec: type: string required: - cluster + - protocol - type type: object type: array diff --git a/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml b/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml index e497fdb3..7a618e54 100644 --- a/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml +++ b/config/crd/bases/worker.kubeslice.io_workersliceconfigs.yaml @@ -117,6 +117,12 @@ spec: sliceCaType: default: Local type: string + sliceGatewayProtocol: + default: UDP + enum: + - TCP + - UDP + type: string sliceGatewayServiceType: default: NodePort enum: diff --git a/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml b/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml index fc78c8c7..8266a588 100644 --- a/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml +++ b/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml @@ -53,6 +53,12 @@ spec: type: string gatewayNumber: type: integer + gatewayProtocol: + default: UDP + enum: + - TCP + - UDP + type: string gatewayType: default: OpenVPN type: string diff --git a/service/kube_slice_resource_names.go b/service/kube_slice_resource_names.go index 03932de3..244a043c 100644 --- a/service/kube_slice_resource_names.go +++ b/service/kube_slice_resource_names.go @@ -151,10 +151,11 @@ var ( ) const ( - serverGateway = "Server" - clientGateway = "Client" - workerSliceGatewayType = "OpenVPN" - defaultSliceGatewayServiceType = "NodePort" + serverGateway = "Server" + clientGateway = "Client" + workerSliceGatewayType = "OpenVPN" + defaultSliceGatewayServiceType = "NodePort" + defaultSliceGatewayServiceProtocol = "UDP" ) var ( diff --git a/service/mocks/IWorkerSliceGatewayService.go b/service/mocks/IWorkerSliceGatewayService.go index 51a827c7..602ed66b 100644 --- a/service/mocks/IWorkerSliceGatewayService.go +++ b/service/mocks/IWorkerSliceGatewayService.go @@ -72,13 +72,13 @@ func (_m *IWorkerSliceGatewayService) DeleteWorkerSliceGatewaysByLabel(ctx conte return r0 } -// GenerateCerts provides a mock function with given fields: ctx, sliceName, namespace, serverGateway, clientGateway, gatewayAddresses -func (_m *IWorkerSliceGatewayService) GenerateCerts(ctx context.Context, sliceName string, namespace string, serverGateway *workerv1alpha1.WorkerSliceGateway, clientGateway *workerv1alpha1.WorkerSliceGateway, gatewayAddresses util.WorkerSliceGatewayNetworkAddresses) error { - ret := _m.Called(ctx, sliceName, namespace, serverGateway, clientGateway, gatewayAddresses) +// GenerateCerts provides a mock function with given fields: ctx, sliceName, namespace, gatewayProtocol, serverGateway, clientGateway, gatewayAddresses +func (_m *IWorkerSliceGatewayService) GenerateCerts(ctx context.Context, sliceName string, namespace string, gatewayProtocol string, serverGateway *workerv1alpha1.WorkerSliceGateway, clientGateway *workerv1alpha1.WorkerSliceGateway, gatewayAddresses util.WorkerSliceGatewayNetworkAddresses) error { + ret := _m.Called(ctx, sliceName, namespace, gatewayProtocol, serverGateway, clientGateway, gatewayAddresses) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, *workerv1alpha1.WorkerSliceGateway, *workerv1alpha1.WorkerSliceGateway, util.WorkerSliceGatewayNetworkAddresses) error); ok { - r0 = rf(ctx, sliceName, namespace, serverGateway, clientGateway, gatewayAddresses) + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, *workerv1alpha1.WorkerSliceGateway, *workerv1alpha1.WorkerSliceGateway, util.WorkerSliceGatewayNetworkAddresses) error); ok { + r0 = rf(ctx, sliceName, namespace, gatewayProtocol, serverGateway, clientGateway, gatewayAddresses) } else { r0 = ret.Error(0) } diff --git a/service/service_helper.go b/service/service_helper.go index 61727e6c..092848a7 100644 --- a/service/service_helper.go +++ b/service/service_helper.go @@ -20,6 +20,7 @@ import ( "context" "time" + "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" "github.com/kubeslice/kubeslice-controller/util" "go.uber.org/zap" ctrl "sigs.k8s.io/controller-runtime" @@ -65,3 +66,19 @@ func RemoveWorkerFinalizers(ctx context.Context, object client.Object, workerFin } return result } + +// get Slice gateway service type for each cluster registered with given slice +func getSliceGwSvcTypes(sliceConfig *v1alpha1.SliceConfig) map[string]*v1alpha1.SliceGatewayServiceType { + var sliceGwSvcTypeMap = make(map[string]*v1alpha1.SliceGatewayServiceType) + for i := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { + gwSvctype := &sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType[i] + if gwSvctype.Cluster == "*" { + for _, cluster := range sliceConfig.Spec.Clusters { + sliceGwSvcTypeMap[cluster] = gwSvctype + } + } else { + sliceGwSvcTypeMap[gwSvctype.Cluster] = gwSvctype + } + } + return sliceGwSvcTypeMap +} diff --git a/service/slice_config_service.go b/service/slice_config_service.go index 730ddd1f..5dbcb517 100644 --- a/service/slice_config_service.go +++ b/service/slice_config_service.go @@ -133,17 +133,9 @@ func (s *SliceConfigService) ReconcileSliceConfig(ctx context.Context, req ctrl. clusterCidr := util.FindCIDRByMaxClusters(sliceConfig.Spec.MaxClusters) completeResourceName := fmt.Sprintf(util.LabelValue, util.GetObjectKind(sliceConfig), sliceConfig.GetName()) ownershipLabel := util.GetOwnerLabel(completeResourceName) - // collect cluster wise slice gw svc info - var sliceGwSvcTypeMap = make(map[string]*v1alpha1.SliceGatewayServiceType) - for _, gwSvctype := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { - if gwSvctype.Cluster == "*" { - for _, cluster := range sliceConfig.Spec.Clusters { - sliceGwSvcTypeMap[cluster] = &gwSvctype - } - } else { - sliceGwSvcTypeMap[gwSvctype.Cluster] = &gwSvctype - } - } + // collect slice gw svc info for given clusters + sliceGwSvcTypeMap := getSliceGwSvcTypes(sliceConfig) + clusterMap, err := s.ms.CreateMinimalWorkerSliceConfig(ctx, sliceConfig.Spec.Clusters, req.Namespace, ownershipLabel, sliceConfig.Name, sliceConfig.Spec.SliceSubnet, clusterCidr, sliceGwSvcTypeMap) if err != nil { return ctrl.Result{}, err diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index e313d3cf..e5f81cf7 100644 --- a/service/slice_config_webhook_validation.go +++ b/service/slice_config_webhook_validation.go @@ -391,7 +391,7 @@ func preventUpdate(ctx context.Context, sc *controllerv1alpha1.SliceConfig, old // check new config for _, new := range sc.Spec.SliceGatewayProvider.SliceGatewayServiceType { oldType, exists := gwSvcType[new.Cluster] - if exists && new.Type != oldType { + if exists && oldType != defaultSliceGatewayServiceType && new.Type != oldType { return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed") } if !exists && new.Type != defaultSliceGatewayServiceType { diff --git a/service/slice_config_webhook_validation_test.go b/service/slice_config_webhook_validation_test.go index 5ba28282..5c4996a8 100644 --- a/service/slice_config_webhook_validation_test.go +++ b/service/slice_config_webhook_validation_test.go @@ -142,12 +142,23 @@ func UpdateValidateSliceConfig_SliceGatewayServiceType(t *testing.T) { Type: "NodePort", }, } + // loadbalancer to nodeport not allowed err := ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig)) require.NotNil(t, err) require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") require.Contains(t, err.Error(), "update not allowed") + + // err := ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig)) + oldSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType[0].Type = "NodePort" + newSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType[0].Type = "LoadBalancer" + err = ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig)) + require.NotNil(t, err) + require.NotContains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") + require.NotContains(t, err.Error(), "update not allowed") + clientMock.AssertExpectations(t) } + func CreateValidateProjectNamespaceDoesNotExist(t *testing.T) { name := "slice_config" namespace := "namespace" diff --git a/service/vpn_key_rotation_service.go b/service/vpn_key_rotation_service.go index 19780a32..7da8437c 100644 --- a/service/vpn_key_rotation_service.go +++ b/service/vpn_key_rotation_service.go @@ -356,7 +356,7 @@ func (v *VpnKeyRotationService) triggerJobsForCertCreation(ctx context.Context, // contruct gw address gatewayAddresses := v.wsgs.BuildNetworkAddresses(s.Spec.SliceSubnet, gateway.Spec.LocalGatewayConfig.ClusterName, gateway.Spec.RemoteGatewayConfig.ClusterName, clusterMap, clusterCidr) // call GenerateCerts() - if err := v.wsgs.GenerateCerts(ctx, s.Name, s.Namespace, &gateway, cl, gatewayAddresses); err != nil { + if err := v.wsgs.GenerateCerts(ctx, s.Name, s.Namespace, gateway.Spec.GatewayProtocol, &gateway, cl, gatewayAddresses); err != nil { return err } } diff --git a/service/vpn_key_rotation_service_test.go b/service/vpn_key_rotation_service_test.go index 2e98ee9f..e302849d 100644 --- a/service/vpn_key_rotation_service_test.go +++ b/service/vpn_key_rotation_service_test.go @@ -589,7 +589,7 @@ func runReconcileVpnKeyRotationConfig(t *testing.T, tc *reconcileVpnKeyRotationC wg.On("BuildNetworkAddresses", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(gwAddress).Once() - wg.On("GenerateCerts", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + wg.On("GenerateCerts", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() clientMock. On("Update", tc.updateArg1, tc.updateArg2).Return(tc.updateRet1).Once() @@ -1103,7 +1103,7 @@ func runTriggerJobsForCertCreation(t *testing.T, tc triggerJobsForCertCreationTe wg.On("BuildNetworkAddresses", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(gwAddress).Once() - wg.On("GenerateCerts", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tc.generateCertsRet1).Once() + wg.On("GenerateCerts", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tc.generateCertsRet1).Once() gotResp := vpn.triggerJobsForCertCreation(ctx, tc.arg1, tc.arg2) require.Equal(t, gotResp, tc.expectedResp) diff --git a/service/worker_slice_config_service.go b/service/worker_slice_config_service.go index 4676faca..f1bbe45e 100644 --- a/service/worker_slice_config_service.go +++ b/service/worker_slice_config_service.go @@ -212,21 +212,18 @@ outer: logger.With(zap.Error(err)).Errorf("Failed to deep copy external gateway configuration") } - // Reconcile Slice gateway service type - sliceGatewayProvider := workerv1alpha1.WorkerSliceGatewayProvider{ - SliceGatewayType: sliceConfig.Spec.SliceGatewayProvider.SliceGatewayType, - SliceCaType: sliceConfig.Spec.SliceGatewayProvider.SliceCaType, - } - gwSvcTypePresent := false - for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { - if gwSvcType.Cluster == "*" || gwSvcType.Cluster == workerSliceConfig.Labels["worker-cluster"] { - sliceGatewayProvider.SliceGatewayServiceType = gwSvcType.Type - gwSvcTypePresent = true - } - } - if !gwSvcTypePresent { - sliceGatewayProvider.SliceGatewayServiceType = defaultSliceGatewayServiceType + // determine Slice gateway service type + sliceGwSvcTypeMap := getSliceGwSvcTypes(sliceConfig) + sliceGwSvcType := defaultSliceGatewayServiceType + sliceGwSvcProtocol := defaultSliceGatewayServiceProtocol + cluster := workerSliceConfig.Labels["worker-cluster"] + if val, exists := sliceGwSvcTypeMap[cluster]; exists { + sliceGwSvcType = val.Type + sliceGwSvcProtocol = val.Protocol } + logger.Debugf("wsc %s reconciler, sliceGwSvcType %s", workerSliceConfig.Name, sliceGwSvcType) + logger.Debugf("slicegwsvctype map in wsc %s", sliceGwSvcTypeMap) + logger.Debugf("wsc reconciler cluster %s, sliceGwProtocol %s", workerSliceConfig.Labels["worker-cluster"], sliceGwSvcProtocol) // Reconcile the Namespace Isolation Profile controllerIsolationProfile := sliceConfig.Spec.NamespaceIsolationProfile @@ -254,7 +251,8 @@ outer: } workerSliceConfig.Spec.ExternalGatewayConfig = externalGatewayConfig - workerSliceConfig.Spec.SliceGatewayProvider = sliceGatewayProvider + workerSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = sliceGwSvcType + workerSliceConfig.Spec.SliceGatewayProvider.SliceGatewayProtocol = sliceGwSvcProtocol workerSliceConfig.Spec.NamespaceIsolationProfile = workerIsolationProfile workerSliceConfig.Spec.SliceName = sliceConfig.Name workerSliceConfig.Spec.Octet = octet @@ -307,9 +305,14 @@ func (s *WorkerSliceConfigService) CreateMinimalWorkerSliceConfig(ctx context.Co clusterSubnetCIDR := fmt.Sprintf(util.GetClusterPrefixPool(sliceSubnet, ipamOctet, clusterCidr)) // determine gw svc type sliceGwSvcType := defaultSliceGatewayServiceType + sliceGwSvcProtocol := defaultSliceGatewayServiceProtocol if val, exists := sliceGwSvcTypeMap[cluster]; exists { sliceGwSvcType = val.Type + sliceGwSvcProtocol = val.Protocol } + logger.Debugf("setting sliceGwSvcType in create_minwsc %s", sliceGwSvcType) + logger.Debugf("setting sliceGwProtocol in create_minwsc %s", sliceGwSvcProtocol) + if !found { label["project-namespace"] = namespace label["original-slice-name"] = name @@ -329,6 +332,7 @@ func (s *WorkerSliceConfigService) CreateMinimalWorkerSliceConfig(ctx context.Co expectedSlice.Spec.ClusterSubnetCIDR = clusterSubnetCIDR expectedSlice.Spec.SliceSubnet = sliceSubnet expectedSlice.Spec.SliceGatewayProvider.SliceGatewayServiceType = sliceGwSvcType + expectedSlice.Spec.SliceGatewayProvider.SliceGatewayProtocol = sliceGwSvcProtocol err = util.CreateResource(ctx, &expectedSlice) if err != nil { //Register an event for worker slice config creation failure @@ -362,6 +366,7 @@ func (s *WorkerSliceConfigService) CreateMinimalWorkerSliceConfig(ctx context.Co existingSlice.Spec.Octet = &ipamOctet existingSlice.Spec.ClusterSubnetCIDR = clusterSubnetCIDR existingSlice.Spec.SliceGatewayProvider.SliceGatewayServiceType = sliceGwSvcType + existingSlice.Spec.SliceGatewayProvider.SliceGatewayProtocol = sliceGwSvcProtocol logger.Debug("updating slice with new octet", existingSlice) if existingSlice.Annotations == nil { existingSlice.Annotations = make(map[string]string) diff --git a/service/worker_slice_gateway_service.go b/service/worker_slice_gateway_service.go index bb6f06a0..9ea74a7a 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -50,7 +50,7 @@ type IWorkerSliceGatewayService interface { ListWorkerSliceGateways(ctx context.Context, ownerLabel map[string]string, namespace string) ([]v1alpha1.WorkerSliceGateway, error) DeleteWorkerSliceGatewaysByLabel(ctx context.Context, label map[string]string, namespace string) error NodeIpReconciliationOfWorkerSliceGateways(ctx context.Context, cluster *controllerv1alpha1.Cluster, namespace string) error - GenerateCerts(ctx context.Context, sliceName string, namespace string, + GenerateCerts(ctx context.Context, sliceName, namespace, gatewayProtocol string, serverGateway *v1alpha1.WorkerSliceGateway, clientGateway *v1alpha1.WorkerSliceGateway, gatewayAddresses util.WorkerSliceGatewayNetworkAddresses) error BuildNetworkAddresses(sliceSubnet, sourceClusterName, destinationClusterName string, @@ -207,21 +207,26 @@ func (s *WorkerSliceGatewayService) ReconcileWorkerSliceGateways(ctx context.Con logger.Infof("sliceConfig %v not found, returning from reconciler loop.", req.NamespacedName) return ctrl.Result{}, nil } - // reconcile gateway connectivity type + + // determine gateway connectivity type & gateway protocol var clusterName string if workerSliceGateway.Spec.GatewayHostType == serverGateway { clusterName = workerSliceGateway.Labels["worker-cluster"] } else { clusterName = workerSliceGateway.Labels["remote-cluster"] } + sliceGwSvcTypeMap := getSliceGwSvcTypes(sliceConfig) gatewayConnectivityType := defaultSliceGatewayServiceType - for _, gwSvcType := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { - if gwSvcType.Cluster == "*" || gwSvcType.Cluster == clusterName { - gatewayConnectivityType = gwSvcType.Type - } + gatewayProtocol := defaultSliceGatewayServiceProtocol + if val, exists := sliceGwSvcTypeMap[clusterName]; exists { + gatewayConnectivityType = val.Type + gatewayProtocol = val.Protocol } workerSliceGateway.Spec.GatewayConnectivityType = gatewayConnectivityType - logger.Debugf("setting gwsvctype %s", workerSliceGateway.Spec.GatewayConnectivityType) + workerSliceGateway.Spec.GatewayProtocol = gatewayProtocol + logger.Debugf("setting gwConType in reconciler %s", workerSliceGateway.Spec.GatewayConnectivityType) + logger.Debugf("setting gwProto in reconciler %s", workerSliceGateway.Spec.GatewayProtocol) + workerSliceGateway.Spec.GatewayType = workerSliceGatewayType workerSliceGateway.UID = "" err = util.UpdateResource(ctx, workerSliceGateway) @@ -435,6 +440,7 @@ func (s *WorkerSliceGatewayService) createMinimumGatewaysIfNotExists(ctx context clusterNames []string, namespace string, ownerLabel map[string]string, clusterMap map[string]int, sliceSubnet string, clusterCidr string, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType) (ctrl.Result, error) { noClusters := len(clusterNames) + logger := util.CtxLogger(ctx) clusterMapping := map[string]*controllerv1alpha1.Cluster{} for _, clusterName := range clusterNames { cluster := controllerv1alpha1.Cluster{} @@ -451,10 +457,14 @@ func (s *WorkerSliceGatewayService) createMinimumGatewaysIfNotExists(ctx context gatewayAddresses := s.BuildNetworkAddresses(sliceSubnet, sourceCluster.Name, destinationCluster.Name, clusterMap, clusterCidr) // determine the gateway svc parameters sliceGwSvcType := defaultSliceGatewayServiceType + gwSvcProtocol := defaultSliceGatewayServiceProtocol if val, exists := sliceGwSvcTypeMap[sourceCluster.Name]; exists { sliceGwSvcType = val.Type + gwSvcProtocol = val.Protocol } - err := s.createMinimumGateWayPairIfNotExists(ctx, sourceCluster, destinationCluster, sliceName, namespace, sliceGwSvcType, ownerLabel, gatewayNumber, gatewayAddresses) + logger.Debugf("setting gwConType in create_minwsg %s", sliceGwSvcType) + logger.Debugf("setting gwProto in create_minwsg %s", gwSvcProtocol) + err := s.createMinimumGateWayPairIfNotExists(ctx, sourceCluster, destinationCluster, sliceName, namespace, sliceGwSvcType, gwSvcProtocol, ownerLabel, gatewayNumber, gatewayAddresses) if err != nil { return ctrl.Result{}, err } @@ -467,7 +477,7 @@ func (s *WorkerSliceGatewayService) createMinimumGatewaysIfNotExists(ctx context // createMinimumGateWayPairIfNotExists is a function to create the pair of gatways between 2 clusters if not exists func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx context.Context, sourceCluster *controllerv1alpha1.Cluster, destinationCluster *controllerv1alpha1.Cluster, - sliceName, namespace, sliceGwSvcType string, label map[string]string, gatewayNumber int, + sliceName, namespace, gatewayConnType, gatewayProtocol string, label map[string]string, gatewayNumber int, gatewayAddresses util.WorkerSliceGatewayNetworkAddresses) error { serverGatewayName := fmt.Sprintf(gatewayName, sliceName, sourceCluster.Name, destinationCluster.Name) clientGatewayName := fmt.Sprintf(gatewayName, sliceName, destinationCluster.Name, sourceCluster.Name) @@ -499,7 +509,10 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont WithNamespace(namespace). WithSlice(sliceName) - serverGatewayObject := s.buildMinimumGateway(sourceCluster, destinationCluster, sliceName, namespace, serverGateway, sliceGwSvcType, label, gatewayNumber, gatewayAddresses.ServerSubnet, gatewayAddresses.ServerVpnAddress, clientGatewayName, gatewayAddresses.ClientSubnet, gatewayAddresses.ClientVpnAddress, serverGatewayName) + serverGatewayObject := s.buildMinimumGateway(sourceCluster, destinationCluster, sliceName, namespace, + serverGateway, gatewayConnType, gatewayProtocol, label, gatewayNumber, + gatewayAddresses.ServerSubnet, gatewayAddresses.ServerVpnAddress, + clientGatewayName, gatewayAddresses.ClientSubnet, gatewayAddresses.ClientVpnAddress, serverGatewayName) err = util.CreateResource(ctx, serverGatewayObject) if err != nil { //Register an event for worker slice gateway creation failure @@ -524,7 +537,10 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont "object_kind": metricKindWorkerSliceGateway, }, ) - clientGatewayObject := s.buildMinimumGateway(destinationCluster, sourceCluster, sliceName, namespace, clientGateway, sliceGwSvcType, label, gatewayNumber, gatewayAddresses.ClientSubnet, gatewayAddresses.ClientVpnAddress, serverGatewayName, gatewayAddresses.ServerSubnet, gatewayAddresses.ServerVpnAddress, clientGatewayName) + clientGatewayObject := s.buildMinimumGateway(destinationCluster, sourceCluster, sliceName, namespace, + clientGateway, gatewayConnType, gatewayProtocol, label, gatewayNumber, + gatewayAddresses.ClientSubnet, gatewayAddresses.ClientVpnAddress, + serverGatewayName, gatewayAddresses.ServerSubnet, gatewayAddresses.ServerVpnAddress, clientGatewayName) err = util.CreateResource(ctx, clientGatewayObject) if err != nil { //Register an event for worker slice gateway creation failure @@ -550,7 +566,7 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont }, ) - err = s.GenerateCerts(ctx, sliceName, namespace, serverGatewayObject, clientGatewayObject, gatewayAddresses) + err = s.GenerateCerts(ctx, sliceName, namespace, gatewayProtocol, serverGatewayObject, clientGatewayObject, gatewayAddresses) if err != nil { return err } @@ -577,7 +593,7 @@ func (s *WorkerSliceGatewayService) BuildNetworkAddresses(sliceSubnet, sourceClu // buildMinimumGateway function returns the gateway object func (s *WorkerSliceGatewayService) buildMinimumGateway(sourceCluster, destinationCluster *controllerv1alpha1.Cluster, - sliceName, namespace, gatewayHostType, sliceGwSvcType string, labels map[string]string, gatewayNumber int, + sliceName, namespace, gatewayHostType, gatewayConnType, gatewayProtocol string, labels map[string]string, gatewayNumber int, gatewaySubnet, localVpnAddress, remoteGatewayName, remoteGatewaySubnet, remoteVpnAddress, localGatewayName string) *v1alpha1.WorkerSliceGateway { labels["worker-cluster"] = sourceCluster.Name labels["remote-cluster"] = destinationCluster.Name @@ -624,13 +640,14 @@ func (s *WorkerSliceGatewayService) buildMinimumGateway(sourceCluster, destinati }, GatewayHostType: gatewayHostType, GatewayNumber: gatewayNumber, - GatewayConnectivityType: sliceGwSvcType, + GatewayConnectivityType: gatewayConnType, + GatewayProtocol: gatewayProtocol, }, } } // generateCerts is a function to generate the certificates between serverGateway and clientGateway -func (s *WorkerSliceGatewayService) GenerateCerts(ctx context.Context, sliceName string, namespace string, +func (s *WorkerSliceGatewayService) GenerateCerts(ctx context.Context, sliceName, namespace, gatewayProtocol string, serverGateway *v1alpha1.WorkerSliceGateway, clientGateway *v1alpha1.WorkerSliceGateway, gatewayAddresses util.WorkerSliceGatewayNetworkAddresses) error { sliceConfig := &controllerv1alpha1.SliceConfig{} @@ -658,8 +675,9 @@ func (s *WorkerSliceGatewayService) GenerateCerts(ctx context.Context, sliceName WithNamespace(namespace). WithSlice(sliceName) - environment := make(map[string]string, 5) + environment := make(map[string]string, 6) environment["NAMESPACE"] = namespace + environment["GATEWAY_PROTOCOL"] = gatewayProtocol environment["SERVER_SLICEGATEWAY_NAME"] = serverGateway.Name environment["CLIENT_SLICEGATEWAY_NAME"] = clientGateway.Name environment["SLICE_NAME"] = sliceName From 442a966cbbb281a8a93f80cd06f0befc0b8a464f Mon Sep 17 00:00:00 2001 From: Mridul Date: Fri, 10 Nov 2023 15:51:52 +0530 Subject: [PATCH 10/13] fix: edge case Signed-off-by: Mridul --- service/slice_config_webhook_validation.go | 4 +--- service/slice_config_webhook_validation_test.go | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index e5f81cf7..11b9f376 100644 --- a/service/slice_config_webhook_validation.go +++ b/service/slice_config_webhook_validation.go @@ -391,12 +391,10 @@ func preventUpdate(ctx context.Context, sc *controllerv1alpha1.SliceConfig, old // check new config for _, new := range sc.Spec.SliceGatewayProvider.SliceGatewayServiceType { oldType, exists := gwSvcType[new.Cluster] + // allow user to update NodePort to LoadBalancer but not vice versa if exists && oldType != defaultSliceGatewayServiceType && new.Type != oldType { return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed") } - if !exists && new.Type != defaultSliceGatewayServiceType { - return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed") - } } return nil } diff --git a/service/slice_config_webhook_validation_test.go b/service/slice_config_webhook_validation_test.go index 5c4996a8..17c71dee 100644 --- a/service/slice_config_webhook_validation_test.go +++ b/service/slice_config_webhook_validation_test.go @@ -148,7 +148,7 @@ func UpdateValidateSliceConfig_SliceGatewayServiceType(t *testing.T) { require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") require.Contains(t, err.Error(), "update not allowed") - // err := ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig)) + // NodePort to LB allowed oldSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType[0].Type = "NodePort" newSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType[0].Type = "LoadBalancer" err = ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig)) From e34420c2bf557afae03836ffca6a79b5fa38d40f Mon Sep 17 00:00:00 2001 From: Mridul Date: Tue, 21 Nov 2023 15:39:31 +0530 Subject: [PATCH 11/13] feat(AM-12018): unit test for getSliceGwSvcTypes() Signed-off-by: Mridul --- service/service_helper_test.go | 89 ++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 service/service_helper_test.go diff --git a/service/service_helper_test.go b/service/service_helper_test.go new file mode 100644 index 00000000..686c447c --- /dev/null +++ b/service/service_helper_test.go @@ -0,0 +1,89 @@ +package service + +import ( + "testing" + + "github.com/dailymotion/allure-go" + "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" + "github.com/stretchr/testify/require" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestServiceHelperSuite(t *testing.T) { + for k, v := range ServiceHelperTestBed { + t.Run(k, func(t *testing.T) { + allure.Test(t, allure.Name(k), + allure.Action(func() { + v(t) + })) + }) + } +} + +var ServiceHelperTestBed = map[string]func(*testing.T){ + "SliceGatewayServiceMap generation": test_getSliceGwSvcTypes, + "SliceGatewayServiceMap generation with wildcard": test_getSliceGwSvcTypes_wildcard, +} + +func test_getSliceGwSvcTypes(t *testing.T) { + sliceConfig := &v1alpha1.SliceConfig{ + ObjectMeta: v1.ObjectMeta{ + Name: "test-slice", + Namespace: "test-ns", + }, + Spec: v1alpha1.SliceConfigSpec{ + SliceGatewayProvider: v1alpha1.WorkerSliceGatewayProvider{ + SliceGatewayServiceType: []v1alpha1.SliceGatewayServiceType{ + { + Cluster: "c1", + Type: "Nodeport", + Protocol: "TCP", + }, + { + Cluster: "c2", + Type: "LoadBalancer", + Protocol: "UDP", + }, + }, + }, + }, + } + sliceGwSvcMap := getSliceGwSvcTypes(sliceConfig) + // assertions + require.Equal(t, len(sliceGwSvcMap), 2) + require.Contains(t, sliceGwSvcMap, "c1") + require.Contains(t, sliceGwSvcMap, "c2") + require.EqualValues(t, &v1alpha1.SliceGatewayServiceType{Cluster: "c1", Type: "Nodeport", Protocol: "TCP"}, sliceGwSvcMap["c1"]) + require.EqualValues(t, &v1alpha1.SliceGatewayServiceType{Cluster: "c2", Type: "LoadBalancer", Protocol: "UDP"}, sliceGwSvcMap["c2"]) +} + +func test_getSliceGwSvcTypes_wildcard(t *testing.T) { + sliceConfig := &v1alpha1.SliceConfig{ + ObjectMeta: v1.ObjectMeta{ + Name: "test-slice", + Namespace: "test-ns", + }, + Spec: v1alpha1.SliceConfigSpec{ + Clusters: []string{ + "c1", "c2", "cx", + }, + SliceGatewayProvider: v1alpha1.WorkerSliceGatewayProvider{ + SliceGatewayServiceType: []v1alpha1.SliceGatewayServiceType{ + { + Cluster: "*", + Type: "Nodeport", + }, + }, + }, + }, + } + sliceGwSvcMap := getSliceGwSvcTypes(sliceConfig) + // assertions + require.Equal(t, len(sliceGwSvcMap), 3) + require.Contains(t, sliceGwSvcMap, "c1") + require.Contains(t, sliceGwSvcMap, "c2") + require.Contains(t, sliceGwSvcMap, "cx") + for _, cluster := range []string{"c1", "c2", "cx"} { + require.EqualValues(t, &v1alpha1.SliceGatewayServiceType{Cluster: "*", Type: "Nodeport"}, sliceGwSvcMap[cluster]) + } +} From 19ecacde525b27f7efd14f3c673a7eb7ac8ea23e Mon Sep 17 00:00:00 2001 From: Mridul Date: Wed, 22 Nov 2023 13:05:41 +0530 Subject: [PATCH 12/13] fix(AM-12018): prevent protocol update Signed-off-by: Mridul --- service/slice_config_webhook_validation.go | 18 ++++---- .../slice_config_webhook_validation_test.go | 44 +++++++++++++------ 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index 11b9f376..bc88cfba 100644 --- a/service/slice_config_webhook_validation.go +++ b/service/slice_config_webhook_validation.go @@ -382,18 +382,20 @@ func preventUpdate(ctx context.Context, sc *controllerv1alpha1.SliceConfig, old return field.Invalid(field.NewPath("Spec").Child("VPNConfig").Child("Cipher"), sc.Spec.VPNConfig.Cipher, "cannot be updated") } } - // can't switch gw svc types - gwSvcType := map[string]string{} + // not allowed to switch gw svc types & protocols // create cluster:GwType map from old config - for _, i := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { - gwSvcType[i.Cluster] = i.Type - } + gwSvcTypeMap := getSliceGwSvcTypes(sliceConfig) + // check new config for _, new := range sc.Spec.SliceGatewayProvider.SliceGatewayServiceType { - oldType, exists := gwSvcType[new.Cluster] + oldType, exists := gwSvcTypeMap[new.Cluster] // allow user to update NodePort to LoadBalancer but not vice versa - if exists && oldType != defaultSliceGatewayServiceType && new.Type != oldType { - return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed") + if exists && oldType.Type != defaultSliceGatewayServiceType && new.Type != oldType.Type { + return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "updating gateway service type is not allowed") + } + // don't allow user to update TCP to UDP & vice versa + if exists && new.Protocol != oldType.Protocol { + return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "updating gateway protocol is not allowed") } } return nil diff --git a/service/slice_config_webhook_validation_test.go b/service/slice_config_webhook_validation_test.go index 17c71dee..422d588c 100644 --- a/service/slice_config_webhook_validation_test.go +++ b/service/slice_config_webhook_validation_test.go @@ -89,7 +89,7 @@ var SliceConfigWebhookValidationTestBed = map[string]func(*testing.T){ "SliceConfigWebhookValidation_UpdateValidateSliceConfigWithExternalGatewayConfigHasAsterisksInMoreThanOnePlace": UpdateValidateSliceConfigWithExternalGatewayConfigHasAsterisksInMoreThanOnePlace, "SliceConfigWebhookValidation_UpdateValidateSliceConfigWithExternalGatewayConfigHasDuplicateClusters": UpdateValidateSliceConfigWithExternalGatewayConfigHasDuplicateClusters, "SliceConfigWebhookValidation_UpdateValidateSliceConfigWithoutErrors": UpdateValidateSliceConfigWithoutErrors, - "SliceConfigWebhookValidation_UpdateValidateSliceGatewayServiceType": UpdateValidateSliceConfig_SliceGatewayServiceType, + "SliceConfigWebhookValidation_UpdateValidateSliceGatewayServiceType": UpdateValidateSliceConfig_PreventUpdate_SliceGatewayServiceType, "SliceConfigWebhookValidation_DeleteValidateSliceConfigWithApplicationNamespacesNotEmpty": DeleteValidateSliceConfigWithApplicationNamespacesAndAllowedNamespacesNotEmpty, "SliceConfigWebhookValidation_DeleteValidateSliceConfigWithOnboardedAppNamespacesNotEmpty": DeleteValidateSliceConfigWithOnboardedAppNamespacesNotEmpty, "SliceConfigWebhookValidation_validateAllowedNamespacesWithDuplicateClusters": ValidateAllowedNamespacesWithDuplicateClusters, @@ -120,7 +120,7 @@ var SliceConfigWebhookValidationTestBed = map[string]func(*testing.T){ "SliceConfigWebhookValidation_UpdateValidateSliceConfigUpdatingVPNCipher": UpdateValidateSliceConfigUpdatingVPNCipher, } -func UpdateValidateSliceConfig_SliceGatewayServiceType(t *testing.T) { +func UpdateValidateSliceConfig_PreventUpdate_SliceGatewayServiceType(t *testing.T) { name := "test-slice" namespace := "demons" oldSliceConfig := controllerv1alpha1.SliceConfig{ @@ -128,11 +128,16 @@ func UpdateValidateSliceConfig_SliceGatewayServiceType(t *testing.T) { Name: name, Namespace: namespace, }, - } - oldSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ - { - Cluster: "c1", - Type: "LoadBalancer", + Spec: controllerv1alpha1.SliceConfigSpec{ + SliceGatewayProvider: controllerv1alpha1.WorkerSliceGatewayProvider{ + SliceGatewayServiceType: []controllerv1alpha1.SliceGatewayServiceType{ + { + Cluster: "c1", + Type: "LoadBalancer", + }, + }, + }, + Clusters: []string{"c1"}, }, } clientMock, newSliceConfig, ctx := setupSliceConfigWebhookValidationTest(name, namespace) @@ -142,19 +147,30 @@ func UpdateValidateSliceConfig_SliceGatewayServiceType(t *testing.T) { Type: "NodePort", }, } + newSliceConfig.Spec.Clusters = []string{"c1"} // loadbalancer to nodeport not allowed err := ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig)) require.NotNil(t, err) require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") - require.Contains(t, err.Error(), "update not allowed") + require.Contains(t, err.Error(), "updating gateway service type is not allowed") - // NodePort to LB allowed - oldSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType[0].Type = "NodePort" - newSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType[0].Type = "LoadBalancer" - err = ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig)) + // tcp to udp & vice-versa not allowed + oldSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ + { + Cluster: "c1", + Protocol: "TCP", + }, + } + newSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ + { + Cluster: "c1", + Protocol: "UDP", + }, + } require.NotNil(t, err) - require.NotContains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") - require.NotContains(t, err.Error(), "update not allowed") + err = ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig)) + require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") + require.Contains(t, err.Error(), "updating gateway protocol is not allowed") clientMock.AssertExpectations(t) } From eb71e9be3be1ed66ea6a9e5bb812e72ece28e451 Mon Sep 17 00:00:00 2001 From: Mridul Date: Wed, 22 Nov 2023 14:55:44 +0530 Subject: [PATCH 13/13] fix(AM-12018): unit tests for validateSlicegatewayServiceType Signed-off-by: Mridul --- service/slice_config_webhook_validation.go | 6 +- .../slice_config_webhook_validation_test.go | 88 ++++++++++++++++++- 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index bc88cfba..ae46d71e 100644 --- a/service/slice_config_webhook_validation.go +++ b/service/slice_config_webhook_validation.go @@ -344,15 +344,15 @@ func validateSlicegatewayServiceType(ctx context.Context, sliceConfig *controlle freq[cluster] += 1 // cluster name can't be empty if cluster == "" { - return field.Invalid(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType").Child("Cluster"), cluster, "Cluster name can't be empty") + return field.Invalid(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), sliceGwSvcType, "Cluster name can't be empty") } // cluster should participate in slice if cluster != "*" && !util.ContainsString(sliceConfig.Spec.Clusters, cluster) { - return field.Invalid(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType").Child("Cluster"), cluster, "Cluster is not participating in slice config") + return field.Invalid(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), sliceGwSvcType, "Cluster is not participating in slice config") } // don't allow duplicate cluster values if freq[cluster] > 1 { - return field.Invalid(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType").Child("Cluster"), cluster, "Duplicate entries are not allowed") + return field.Invalid(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), sliceGwSvcType, "Duplicate entries for same cluster are not allowed") } } return nil diff --git a/service/slice_config_webhook_validation_test.go b/service/slice_config_webhook_validation_test.go index 422d588c..7050ab43 100644 --- a/service/slice_config_webhook_validation_test.go +++ b/service/slice_config_webhook_validation_test.go @@ -118,11 +118,95 @@ var SliceConfigWebhookValidationTestBed = map[string]func(*testing.T){ "TestValidateRotationInterval_Change_Increased": TestValidateRotationInterval_Change_Increased, "TestValidateRotationInterval_NoChange": TestValidateRotationInterval_NoChange, "SliceConfigWebhookValidation_UpdateValidateSliceConfigUpdatingVPNCipher": UpdateValidateSliceConfigUpdatingVPNCipher, + "Test_validateSlicegatewayServiceType": test_validateSlicegatewayServiceType, +} + +func test_validateSlicegatewayServiceType(t *testing.T) { + name := "test-slice" + namespace := "test-ns" + clientMock, sliceConfig, ctx := setupSliceConfigWebhookValidationTest(name, namespace) + // slicegatewayServiceType definition is optional + err := validateSlicegatewayServiceType(ctx, sliceConfig) + require.Nil(t, err) + clientMock.AssertExpectations(t) + + // if defined, cluster name can't be empty + sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ + { + Type: "Loadbalancer", + Protocol: "UDP", + }, + } + err = validateSlicegatewayServiceType(ctx, sliceConfig) + require.NotNil(t, err) + require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Invalid value:") + require.Contains(t, err.Error(), "Cluster name can't be empty") + clientMock.AssertExpectations(t) + + // if defined, cluster name should be part of slice + sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ + { + Cluster: "demo-cluster", + Type: "Loadbalancer", + Protocol: "UDP", + }, + } + err = validateSlicegatewayServiceType(ctx, sliceConfig) + require.NotNil(t, err) + require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Invalid value:") + require.Contains(t, err.Error(), "Cluster is not participating in slice config") + clientMock.AssertExpectations(t) + + // if defined, cluster name should be part of slice + sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ + { + Cluster: "demo-cluster", + Type: "Loadbalancer", + Protocol: "UDP", + }, + { + Cluster: "demo-cluster", + Type: "Nodeport", + Protocol: "TCP", + }, + } + sliceConfig.Spec.Clusters = []string{"demo-cluster"} + err = validateSlicegatewayServiceType(ctx, sliceConfig) + require.NotNil(t, err) + require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Invalid value:") + require.Contains(t, err.Error(), "Duplicate entries for same cluster are not allowed") + clientMock.AssertExpectations(t) + + // happy scenario: all check passes + sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ + { + Cluster: "demo-cluster", + Type: "Loadbalancer", + Protocol: "UDP", + }, + } + sliceConfig.Spec.Clusters = []string{"demo-cluster"} + err = validateSlicegatewayServiceType(ctx, sliceConfig) + require.Nil(t, err) + clientMock.AssertExpectations(t) + + // happy scenario with wild card: all check passes + sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ + { + Cluster: "*", + Type: "Loadbalancer", + Protocol: "UDP", + }, + } + sliceConfig.Spec.Clusters = []string{"demo-cluster", "c2", "cx"} + err = validateSlicegatewayServiceType(ctx, sliceConfig) + require.Nil(t, err) + clientMock.AssertExpectations(t) } func UpdateValidateSliceConfig_PreventUpdate_SliceGatewayServiceType(t *testing.T) { name := "test-slice" - namespace := "demons" + namespace := "test-ns" oldSliceConfig := controllerv1alpha1.SliceConfig{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -153,6 +237,7 @@ func UpdateValidateSliceConfig_PreventUpdate_SliceGatewayServiceType(t *testing. require.NotNil(t, err) require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") require.Contains(t, err.Error(), "updating gateway service type is not allowed") + clientMock.AssertExpectations(t) // tcp to udp & vice-versa not allowed oldSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ @@ -171,7 +256,6 @@ func UpdateValidateSliceConfig_PreventUpdate_SliceGatewayServiceType(t *testing. err = ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig)) require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") require.Contains(t, err.Error(), "updating gateway protocol is not allowed") - clientMock.AssertExpectations(t) }