From 30526f4ee4a585e8e60066feaaf47497a4a64904 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Sun, 5 Jul 2026 12:18:31 +0530 Subject: [PATCH 01/21] feat(): add hub-and-spoke topology fields to SliceConfig CRD Signed-off-by: Shreesha001 --- apis/controller/v1alpha1/sliceconfig_types.go | 25 ++++ .../v1alpha1/zz_generated.deepcopy.go | 25 ++++ .../controller.kubeslice.io_sliceconfigs.yaml | 24 +++- ...er_v1alpha1_sliceconfig_hub_and_spoke.yaml | 35 ++++++ service/slice_config_webhook_validation.go | 69 +++++++++++ .../slice_config_webhook_validation_test.go | 111 ++++++++++++++++++ 6 files changed, 288 insertions(+), 1 deletion(-) create mode 100644 config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml diff --git a/apis/controller/v1alpha1/sliceconfig_types.go b/apis/controller/v1alpha1/sliceconfig_types.go index 38b7bbde..02140b05 100644 --- a/apis/controller/v1alpha1/sliceconfig_types.go +++ b/apis/controller/v1alpha1/sliceconfig_types.go @@ -74,6 +74,31 @@ type SliceConfigSpec struct { // RenewBefore is used for renew now! RenewBefore *metav1.Time `json:"renewBefore,omitempty"` VPNConfig *VPNConfiguration `json:"vpnConfig,omitempty"` + // Topology configures the inter-cluster connection topology for the slice. + // When absent, the slice uses full-mesh connectivity (existing behavior). + //+optional + Topology *TopologySpec `json:"topology,omitempty"` +} + +// +kubebuilder:validation:Enum=FullMesh;HubAndSpoke +type TopologyMode string + +const ( + TopologyModeFullMesh TopologyMode = "FullMesh" + TopologyModeHubAndSpoke TopologyMode = "HubAndSpoke" +) + +// TopologySpec defines the inter-cluster connection topology of the slice +type TopologySpec struct { + // Mode selects the connection topology. Absent defaults to FullMesh. + //+optional + Mode TopologyMode `json:"mode,omitempty"` + // Hubs lists the clusters acting as hubs when Mode is HubAndSpoke. + // Each entry must be a member of spec.clusters. All non-hub members + // become spokes. + //+optional + //+kubebuilder:validation:MaxItems=2 + Hubs []string `json:"hubs,omitempty"` } // ExternalGatewayConfig is the configuration for external gateways like 'istio', etc/ diff --git a/apis/controller/v1alpha1/zz_generated.deepcopy.go b/apis/controller/v1alpha1/zz_generated.deepcopy.go index a7f442c9..9a6d13e2 100644 --- a/apis/controller/v1alpha1/zz_generated.deepcopy.go +++ b/apis/controller/v1alpha1/zz_generated.deepcopy.go @@ -764,6 +764,11 @@ func (in *SliceConfigSpec) DeepCopyInto(out *SliceConfigSpec) { *out = new(VPNConfiguration) **out = **in } + if in.Topology != nil { + in, out := &in.Topology, &out.Topology + *out = new(TopologySpec) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SliceConfigSpec. @@ -974,6 +979,26 @@ func (in *Telemetry) DeepCopy() *Telemetry { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TopologySpec) DeepCopyInto(out *TopologySpec) { + *out = *in + if in.Hubs != nil { + in, out := &in.Hubs, &out.Hubs + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TopologySpec. +func (in *TopologySpec) DeepCopy() *TopologySpec { + if in == nil { + return nil + } + out := new(TopologySpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VCPURestriction) DeepCopyInto(out *VCPURestriction) { *out = *in diff --git a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml index 00c96917..662f5faa 100644 --- a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml +++ b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.19.0 + controller-gen.kubebuilder.io/version: v0.17.3 name: sliceconfigs.controller.kubeslice.io spec: group: controller.kubeslice.io @@ -227,6 +227,28 @@ spec: type: string standardQosProfileName: type: string + topology: + description: |- + Topology configures the inter-cluster connection topology for the slice. + When absent, the slice uses full-mesh connectivity (existing behavior). + properties: + hubs: + description: |- + Hubs lists the clusters acting as hubs when Mode is HubAndSpoke. + Each entry must be a member of spec.clusters. All non-hub members + become spokes. + items: + type: string + maxItems: 2 + type: array + mode: + description: Mode selects the connection topology. Absent defaults + to FullMesh. + enum: + - FullMesh + - HubAndSpoke + type: string + type: object vpnConfig: description: VPNConfiguration defines the additional (optional) VPN Configuration to customise diff --git a/config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml b/config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml new file mode 100644 index 00000000..f7a8a5c3 --- /dev/null +++ b/config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml @@ -0,0 +1,35 @@ +# Sample SliceConfig using the HubAndSpoke topology. +# +# worker-1 acts as the hub; worker-2 and worker-3 become spokes (all +# non-hub members are spokes). Tunnel links are created only between +# hub and spokes: worker-1<->worker-2 and worker-1<->worker-3. +# No spoke<->spoke (worker-2<->worker-3) link is created. +# +# Omitting spec.topology entirely (or setting mode: FullMesh with no +# hubs) keeps the existing full-mesh behavior. +apiVersion: controller.kubeslice.io/v1alpha1 +kind: SliceConfig +metadata: + name: hub-and-spoke-slice +spec: + sliceType: Application + sliceSubnet: 10.1.0.0/16 + sliceGatewayProvider: + sliceGatewayType: OpenVPN + sliceCaType: Local + sliceIpamType: Local + clusters: + - worker-1 + - worker-2 + - worker-3 + topology: + mode: HubAndSpoke + hubs: + - worker-1 + qosProfileDetails: + queueType: HTB + priority: 1 + tcType: BANDWIDTH_CONTROL + bandwidthCeilingKbps: 5120 + bandwidthGuaranteedKbps: 2560 + dscpClass: AF11 \ No newline at end of file diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index 99ba653a..ae38d0f8 100644 --- a/service/slice_config_webhook_validation.go +++ b/service/slice_config_webhook_validation.go @@ -59,6 +59,9 @@ func ValidateSliceConfigCreate(ctx context.Context, sliceConfig *controllerv1alp if err := validateMaxClusterCount(sliceConfig); err != nil { return nil, apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "SliceConfig"}, sliceConfig.Name, field.ErrorList{err}) } + if err := validateTopology(sliceConfig); err != nil { + return nil, apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "SliceConfig"}, sliceConfig.Name, field.ErrorList{err}) + } if sliceConfig.Spec.OverlayNetworkDeploymentMode != controllerv1alpha1.NONET { if err := validateSliceSubnet(sliceConfig); err != nil { return nil, apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "SliceConfig"}, sliceConfig.Name, field.ErrorList{err}) @@ -106,6 +109,9 @@ func ValidateSliceConfigUpdate(ctx context.Context, sliceConfig *controllerv1alp if err := validateNamespaceIsolationProfile(sliceConfig); err != nil { return nil, apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "SliceConfig"}, sliceConfig.Name, field.ErrorList{err}) } + if err := validateTopology(sliceConfig); err != nil { + return nil, apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "SliceConfig"}, sliceConfig.Name, field.ErrorList{err}) + } // Validate single/multi overlay network deployment mode specific fields if sliceConfig.Spec.OverlayNetworkDeploymentMode != controllerv1alpha1.NONET { if err := validateSliceSubnet(sliceConfig); err != nil { @@ -331,6 +337,69 @@ func validateClustersOnCreate(ctx context.Context, sliceConfig *controllerv1alph return nil } +// validateTopology is function to validate the topology specification of slice config +func validateTopology(sliceConfig *controllerv1alpha1.SliceConfig) *field.Error { + topology := sliceConfig.Spec.Topology + if topology == nil { + return nil + } + topologyPath := field.NewPath("Spec").Child("Topology") + switch topology.Mode { + case "": + if len(topology.Hubs) > 0 { + return field.Required(topologyPath.Child("Mode"), "mode must be set to HubAndSpoke when hubs is specified") + } + return nil + case controllerv1alpha1.TopologyModeFullMesh: + if len(topology.Hubs) > 0 { + return field.Invalid(topologyPath.Child("Hubs"), topology.Hubs, "hubs must be empty when mode is FullMesh") + } + return nil + case controllerv1alpha1.TopologyModeHubAndSpoke: + return validateHubAndSpokeTopology(sliceConfig, topologyPath) + default: + return field.Invalid(topologyPath.Child("Mode"), string(topology.Mode), "unknown topology mode; valid values: FullMesh, HubAndSpoke") + } +} + +// validateHubAndSpokeTopology is function to validate the hub and spoke topology rules +func validateHubAndSpokeTopology(sliceConfig *controllerv1alpha1.SliceConfig, topologyPath *field.Path) *field.Error { + topology := sliceConfig.Spec.Topology + if len(sliceConfig.Spec.Clusters) < 2 { + return field.Invalid(topologyPath.Child("Mode"), string(topology.Mode), "HubAndSpoke topology requires at least 2 clusters") + } + if len(topology.Hubs) == 0 { + return field.Required(topologyPath.Child("Hubs"), "HubAndSpoke topology requires at least one hub") + } + if duplicate, value := util.CheckDuplicateInArray(topology.Hubs); duplicate { + return field.Duplicate(topologyPath.Child("Hubs"), "duplicate hub entry: "+strings.Join(value, ", ")) + } + if len(topology.Hubs) > 1 { + return field.Invalid(topologyPath.Child("Hubs"), topology.Hubs, "only one hub is supported in this release") + } + members := make(map[string]bool, len(sliceConfig.Spec.Clusters)) + for _, clusterName := range sliceConfig.Spec.Clusters { + members[clusterName] = true + } + hubs := make(map[string]bool, len(topology.Hubs)) + for i, hubName := range topology.Hubs { + if !members[hubName] { + return field.Invalid(topologyPath.Child("Hubs").Index(i), hubName, "hub is not a member of spec.clusters") + } + hubs[hubName] = true + } + spokes := 0 + for _, clusterName := range sliceConfig.Spec.Clusters { + if !hubs[clusterName] { + spokes++ + } + } + if spokes == 0 { + return field.Invalid(topologyPath.Child("Hubs"), topology.Hubs, "HubAndSpoke topology requires at least one spoke cluster") + } + return nil +} + // validateClustersOnUpdate is function to validate the cluster specification func validateClustersOnUpdate(ctx context.Context, sliceConfig *controllerv1alpha1.SliceConfig, old runtime.Object) *field.Error { oldSc := old.(*controllerv1alpha1.SliceConfig) diff --git a/service/slice_config_webhook_validation_test.go b/service/slice_config_webhook_validation_test.go index 30373811..2f93494e 100644 --- a/service/slice_config_webhook_validation_test.go +++ b/service/slice_config_webhook_validation_test.go @@ -119,6 +119,7 @@ var SliceConfigWebhookValidationTestBed = map[string]func(*testing.T){ "TestValidateRotationInterval_NoChange": TestValidateRotationInterval_NoChange, "SliceConfigWebhookValidation_UpdateValidateSliceConfigUpdatingVPNCipher": UpdateValidateSliceConfigUpdatingVPNCipher, "Test_validateSlicegatewayServiceType": test_validateSlicegatewayServiceType, + "SliceConfigWebhookValidation_validateTopology": test_validateTopology, } func test_validateSlicegatewayServiceType(t *testing.T) { @@ -2316,3 +2317,113 @@ func setupSliceConfigWebhookValidationTest(name string, namespace string) (*util ctx := util.PrepareKubeSliceControllersRequestContext(context.Background(), clientMock, nil, "SliceConfigWebhookValidationServiceTest", nil) return clientMock, sliceConfig, ctx } + +func test_validateTopology(t *testing.T) { + clusters := []string{"cluster-1", "cluster-2", "cluster-3"} + testCases := []struct { + name string + clusters []string + topology *controllerv1alpha1.TopologySpec + wantErr bool + errContains string + }{ + { + name: "absent topology is valid (full mesh default)", + clusters: clusters, + topology: nil, + wantErr: false, + }, + { + name: "explicit FullMesh without hubs is valid", + clusters: clusters, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeFullMesh}, + wantErr: false, + }, + { + name: "valid HubAndSpoke with one hub", + clusters: clusters, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"cluster-1"}}, + wantErr: false, + }, + { + name: "HubAndSpoke without hubs is rejected", + clusters: clusters, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke}, + wantErr: true, + errContains: "requires at least one hub", + }, + { + name: "hub not a member of clusters is rejected", + clusters: clusters, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"other-cluster"}}, + wantErr: true, + errContains: "is not a member of spec.clusters", + }, + { + name: "all clusters as hubs leaves no spokes and is rejected", + clusters: []string{"cluster-1"}, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"cluster-1"}}, + wantErr: true, + errContains: "requires at least 2 clusters", + }, + { + name: "FullMesh with hubs is rejected", + clusters: clusters, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeFullMesh, Hubs: []string{"cluster-1"}}, + wantErr: true, + errContains: "hubs must be empty when mode is FullMesh", + }, + { + name: "duplicate hub entries are rejected", + clusters: clusters, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"cluster-1", "cluster-1"}}, + wantErr: true, + errContains: "duplicate hub entry", + }, + { + name: "unknown mode is rejected", + clusters: clusters, + topology: &controllerv1alpha1.TopologySpec{Mode: "Ring"}, + wantErr: true, + errContains: "unknown topology mode", + }, + { + name: "hubs without mode is rejected", + clusters: clusters, + topology: &controllerv1alpha1.TopologySpec{Hubs: []string{"cluster-1"}}, + wantErr: true, + errContains: "mode must be set to HubAndSpoke when hubs is specified", + }, + { + name: "more than one hub is rejected in this release", + clusters: clusters, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"cluster-1", "cluster-2"}}, + wantErr: true, + errContains: "only one hub is supported", + }, + { + name: "HubAndSpoke with fewer than 2 clusters is rejected", + clusters: []string{"cluster-1"}, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"cluster-1"}}, + wantErr: true, + errContains: "requires at least 2 clusters", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + sliceConfig := &controllerv1alpha1.SliceConfig{ + Spec: controllerv1alpha1.SliceConfigSpec{ + Clusters: tc.clusters, + Topology: tc.topology, + }, + } + err := validateTopology(sliceConfig) + if tc.wantErr { + require.NotNil(t, err) + require.Contains(t, err.Error(), tc.errContains) + } else { + require.Nil(t, err) + } + }) + } +} From 4962128cb9eb6cebb71619c16c030aa72dd18534 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Sun, 12 Jul 2026 19:28:22 +0530 Subject: [PATCH 02/21] address review: single-hub schema cap, doc/comment fixes, regen CRD - pin Hubs to MaxItems=1 (schema now matches the single-hub webhook rule) - order the single-hub check before the duplicate check; mark both as defense-in-depth behind the schema limit - comment the unreachable spokes==0 guard - clarify the sample is API/validation-only (topology not yet consumed) - add trailing newline to sample; regen CRD with controller-gen v0.19.0 Signed-off-by: Shreesha001 --- apis/controller/v1alpha1/sliceconfig_types.go | 4 ++-- .../bases/controller.kubeslice.io_sliceconfigs.yaml | 6 +++--- ...ntroller_v1alpha1_sliceconfig_hub_and_spoke.yaml | 12 ++++++++---- service/slice_config_webhook_validation.go | 13 ++++++++++--- service/slice_config_webhook_validation_test.go | 8 ++++---- 5 files changed, 27 insertions(+), 16 deletions(-) diff --git a/apis/controller/v1alpha1/sliceconfig_types.go b/apis/controller/v1alpha1/sliceconfig_types.go index 02140b05..636befdc 100644 --- a/apis/controller/v1alpha1/sliceconfig_types.go +++ b/apis/controller/v1alpha1/sliceconfig_types.go @@ -95,9 +95,9 @@ type TopologySpec struct { Mode TopologyMode `json:"mode,omitempty"` // Hubs lists the clusters acting as hubs when Mode is HubAndSpoke. // Each entry must be a member of spec.clusters. All non-hub members - // become spokes. + // become spokes. Exactly one hub is supported in this release. //+optional - //+kubebuilder:validation:MaxItems=2 + //+kubebuilder:validation:MaxItems=1 Hubs []string `json:"hubs,omitempty"` } diff --git a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml index 662f5faa..f5591697 100644 --- a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml +++ b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.17.3 + controller-gen.kubebuilder.io/version: v0.19.0 name: sliceconfigs.controller.kubeslice.io spec: group: controller.kubeslice.io @@ -236,10 +236,10 @@ spec: description: |- Hubs lists the clusters acting as hubs when Mode is HubAndSpoke. Each entry must be a member of spec.clusters. All non-hub members - become spokes. + become spokes. Exactly one hub is supported in this release. items: type: string - maxItems: 2 + maxItems: 1 type: array mode: description: Mode selects the connection topology. Absent defaults diff --git a/config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml b/config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml index f7a8a5c3..5e1ccec2 100644 --- a/config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml +++ b/config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml @@ -1,9 +1,13 @@ # Sample SliceConfig using the HubAndSpoke topology. # # worker-1 acts as the hub; worker-2 and worker-3 become spokes (all -# non-hub members are spokes). Tunnel links are created only between -# hub and spokes: worker-1<->worker-2 and worker-1<->worker-3. -# No spoke<->spoke (worker-2<->worker-3) link is created. +# non-hub members are spokes). +# +# Note: this sample exercises the API and validation only. As of this +# release the controller does not yet consume spec.topology to change +# gateway/peer link creation (that is follow-up implementation work); +# the intended result once it does is hub<->spoke links only +# (worker-1<->worker-2, worker-1<->worker-3) and no spoke<->spoke link. # # Omitting spec.topology entirely (or setting mode: FullMesh with no # hubs) keeps the existing full-mesh behavior. @@ -32,4 +36,4 @@ spec: tcType: BANDWIDTH_CONTROL bandwidthCeilingKbps: 5120 bandwidthGuaranteedKbps: 2560 - dscpClass: AF11 \ No newline at end of file + dscpClass: AF11 diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index ae38d0f8..57b07a04 100644 --- a/service/slice_config_webhook_validation.go +++ b/service/slice_config_webhook_validation.go @@ -371,12 +371,17 @@ func validateHubAndSpokeTopology(sliceConfig *controllerv1alpha1.SliceConfig, to if len(topology.Hubs) == 0 { return field.Required(topologyPath.Child("Hubs"), "HubAndSpoke topology requires at least one hub") } - if duplicate, value := util.CheckDuplicateInArray(topology.Hubs); duplicate { - return field.Duplicate(topologyPath.Child("Hubs"), "duplicate hub entry: "+strings.Join(value, ", ")) - } + // The next two checks are defense-in-depth: the CRD schema pins Hubs to + // MaxItems=1, so structural validation already rejects >1 (and therefore + // any duplicate) before admission reaches this webhook. They remain as a + // safety net and stay meaningful if the schema limit is raised for + // multi-hub. The single-hub check is ordered first so it wins when both apply. if len(topology.Hubs) > 1 { return field.Invalid(topologyPath.Child("Hubs"), topology.Hubs, "only one hub is supported in this release") } + if duplicate, value := util.CheckDuplicateInArray(topology.Hubs); duplicate { + return field.Duplicate(topologyPath.Child("Hubs"), "duplicate hub entry: "+strings.Join(value, ", ")) + } members := make(map[string]bool, len(sliceConfig.Spec.Clusters)) for _, clusterName := range sliceConfig.Spec.Clusters { members[clusterName] = true @@ -394,6 +399,8 @@ func validateHubAndSpokeTopology(sliceConfig *controllerv1alpha1.SliceConfig, to spokes++ } } + // spokes == 0 is unreachable today (1 hub max + >=2 clusters => >=1 spoke), + // but guards against regressions when the hub count limit is raised. if spokes == 0 { return field.Invalid(topologyPath.Child("Hubs"), topology.Hubs, "HubAndSpoke topology requires at least one spoke cluster") } diff --git a/service/slice_config_webhook_validation_test.go b/service/slice_config_webhook_validation_test.go index 2f93494e..1e276680 100644 --- a/service/slice_config_webhook_validation_test.go +++ b/service/slice_config_webhook_validation_test.go @@ -2360,7 +2360,7 @@ func test_validateTopology(t *testing.T) { errContains: "is not a member of spec.clusters", }, { - name: "all clusters as hubs leaves no spokes and is rejected", + name: "single-cluster HubAndSpoke is rejected (needs at least 2 clusters)", clusters: []string{"cluster-1"}, topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"cluster-1"}}, wantErr: true, @@ -2374,11 +2374,11 @@ func test_validateTopology(t *testing.T) { errContains: "hubs must be empty when mode is FullMesh", }, { - name: "duplicate hub entries are rejected", + name: "more than one hub is rejected (single-hub MVP)", clusters: clusters, - topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"cluster-1", "cluster-1"}}, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"cluster-1", "cluster-2"}}, wantErr: true, - errContains: "duplicate hub entry", + errContains: "only one hub is supported in this release", }, { name: "unknown mode is rejected", From eea8f1f985ec053eda102b408d65b38edd21a69b Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Mon, 13 Jul 2026 18:06:05 +0530 Subject: [PATCH 03/21] address review: use field.Duplicate value form, drop duplicate topology test cases Signed-off-by: Shreesha001 --- service/slice_config_webhook_validation.go | 2 +- service/slice_config_webhook_validation_test.go | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index 57b07a04..de457f76 100644 --- a/service/slice_config_webhook_validation.go +++ b/service/slice_config_webhook_validation.go @@ -380,7 +380,7 @@ func validateHubAndSpokeTopology(sliceConfig *controllerv1alpha1.SliceConfig, to return field.Invalid(topologyPath.Child("Hubs"), topology.Hubs, "only one hub is supported in this release") } if duplicate, value := util.CheckDuplicateInArray(topology.Hubs); duplicate { - return field.Duplicate(topologyPath.Child("Hubs"), "duplicate hub entry: "+strings.Join(value, ", ")) + return field.Duplicate(topologyPath.Child("Hubs"), strings.Join(value, ", ")) } members := make(map[string]bool, len(sliceConfig.Spec.Clusters)) for _, clusterName := range sliceConfig.Spec.Clusters { diff --git a/service/slice_config_webhook_validation_test.go b/service/slice_config_webhook_validation_test.go index 1e276680..92fc3b34 100644 --- a/service/slice_config_webhook_validation_test.go +++ b/service/slice_config_webhook_validation_test.go @@ -2394,20 +2394,6 @@ func test_validateTopology(t *testing.T) { wantErr: true, errContains: "mode must be set to HubAndSpoke when hubs is specified", }, - { - name: "more than one hub is rejected in this release", - clusters: clusters, - topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"cluster-1", "cluster-2"}}, - wantErr: true, - errContains: "only one hub is supported", - }, - { - name: "HubAndSpoke with fewer than 2 clusters is rejected", - clusters: []string{"cluster-1"}, - topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"cluster-1"}}, - wantErr: true, - errContains: "requires at least 2 clusters", - }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { From 031168291e164017c069a88db5ae82039da81459 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Sun, 12 Jul 2026 21:41:07 +0530 Subject: [PATCH 04/21] feat: add TopologyResolver for hub-and-spoke edge computation Signed-off-by: Shreesha001 --- service/topology_resolver.go | 69 ++++++++++++++++++++++++++ service/topology_resolver_test.go | 80 +++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 service/topology_resolver.go create mode 100644 service/topology_resolver_test.go diff --git a/service/topology_resolver.go b/service/topology_resolver.go new file mode 100644 index 00000000..225bbd82 --- /dev/null +++ b/service/topology_resolver.go @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2026 Avesha, Inc. All rights reserved. # # SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package service + +import ( + controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" +) + +// TopologyEdge is a desired gateway connection between two clusters of a +// slice. ServerCluster hosts the gateway server side of the pair and +// ClientCluster dials in; for hub-and-spoke edges the hub is always the +// server so that spokes behind NAT never need to accept inbound connections. +type TopologyEdge struct { + ServerCluster string + ClientCluster string +} + +// ResolveTopologyEdges computes the desired set of gateway connections for a +// slice from its member clusters and topology configuration. The result is +// deterministic. A nil topology or FullMesh mode yields every cluster pair in +// cluster-list order, matching existing full-mesh behavior. HubAndSpoke yields +// hub<->spoke edges (hub as server) for every hub, plus hub<->hub edges when +// more than one hub is configured. +func ResolveTopologyEdges(clusters []string, topology *controllerv1alpha1.TopologySpec) []TopologyEdge { + edges := []TopologyEdge{} + if topology == nil || topology.Mode != controllerv1alpha1.TopologyModeHubAndSpoke { + for i := 0; i < len(clusters); i++ { + for j := i + 1; j < len(clusters); j++ { + edges = append(edges, TopologyEdge{ServerCluster: clusters[i], ClientCluster: clusters[j]}) + } + } + return edges + } + hubs := make(map[string]bool, len(topology.Hubs)) + for _, hub := range topology.Hubs { + hubs[hub] = true + } + spokes := []string{} + for _, cluster := range clusters { + if !hubs[cluster] { + spokes = append(spokes, cluster) + } + } + for _, hub := range topology.Hubs { + for _, spoke := range spokes { + edges = append(edges, TopologyEdge{ServerCluster: hub, ClientCluster: spoke}) + } + } + for i := 0; i < len(topology.Hubs); i++ { + for j := i + 1; j < len(topology.Hubs); j++ { + edges = append(edges, TopologyEdge{ServerCluster: topology.Hubs[i], ClientCluster: topology.Hubs[j]}) + } + } + return edges +} diff --git a/service/topology_resolver_test.go b/service/topology_resolver_test.go new file mode 100644 index 00000000..27a648b9 --- /dev/null +++ b/service/topology_resolver_test.go @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2026 Avesha, Inc. All rights reserved. # # SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package service + +import ( + "reflect" + "testing" + + controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" +) + +func TestResolveTopologyEdges(t *testing.T) { + cases := []struct { + name string + clusters []string + topology *controllerv1alpha1.TopologySpec + want []TopologyEdge + }{ + { + name: "nil topology is full mesh in cluster order", + clusters: []string{"a", "b", "c"}, + topology: nil, + want: []TopologyEdge{ + {ServerCluster: "a", ClientCluster: "b"}, + {ServerCluster: "a", ClientCluster: "c"}, + {ServerCluster: "b", ClientCluster: "c"}, + }, + }, + { + name: "explicit FullMesh is full mesh", + clusters: []string{"a", "b", "c"}, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeFullMesh}, + want: []TopologyEdge{ + {ServerCluster: "a", ClientCluster: "b"}, + {ServerCluster: "a", ClientCluster: "c"}, + {ServerCluster: "b", ClientCluster: "c"}, + }, + }, + { + name: "hub and spoke: hub is server, no spoke-to-spoke", + clusters: []string{"worker-1", "worker-2", "worker-3"}, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"worker-1"}}, + want: []TopologyEdge{ + {ServerCluster: "worker-1", ClientCluster: "worker-2"}, + {ServerCluster: "worker-1", ClientCluster: "worker-3"}, + }, + }, + { + name: "hub is server even when not first in cluster list", + clusters: []string{"worker-1", "worker-2", "worker-3"}, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"worker-2"}}, + want: []TopologyEdge{ + {ServerCluster: "worker-2", ClientCluster: "worker-1"}, + {ServerCluster: "worker-2", ClientCluster: "worker-3"}, + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := ResolveTopologyEdges(tc.clusters, tc.topology) + if !reflect.DeepEqual(got, tc.want) { + t.Fatalf("%s:\n got %v\n want %v", tc.name, got, tc.want) + } + }) + } +} From 7e7b16f53e7561078258bf3a442b5e9ad9a6134a Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Sun, 12 Jul 2026 22:00:53 +0530 Subject: [PATCH 05/21] feat: gate WorkerSliceGateway creation on resolved topology edges Signed-off-by: Shreesha001 --- service/mocks/IWorkerSliceGatewayService.go | 16 ++--- service/slice_config_service.go | 2 +- service/slice_config_service_test.go | 8 +-- service/worker_slice_gateway_service.go | 64 +++++++++++--------- service/worker_slice_gateway_service_test.go | 4 +- 5 files changed, 49 insertions(+), 45 deletions(-) diff --git a/service/mocks/IWorkerSliceGatewayService.go b/service/mocks/IWorkerSliceGatewayService.go index 602ed66b..205a6b74 100644 --- a/service/mocks/IWorkerSliceGatewayService.go +++ b/service/mocks/IWorkerSliceGatewayService.go @@ -35,22 +35,22 @@ func (_m *IWorkerSliceGatewayService) BuildNetworkAddresses(sliceSubnet string, } // 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) +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, topology *v1alpha1.TopologySpec) (reconcile.Result, error) { + ret := _m.Called(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap, topology) 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, 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, map[string]*v1alpha1.SliceGatewayServiceType, *v1alpha1.TopologySpec) (reconcile.Result, error)); ok { + return rf(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap, topology) } - 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) + if rf, ok := ret.Get(0).(func(context.Context, string, []string, string, map[string]string, map[string]int, string, string, map[string]*v1alpha1.SliceGatewayServiceType, *v1alpha1.TopologySpec) reconcile.Result); ok { + r0 = rf(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap, topology) } 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, map[string]*v1alpha1.SliceGatewayServiceType) error); ok { - r1 = rf(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) + if rf, ok := ret.Get(1).(func(context.Context, string, []string, string, map[string]string, map[string]int, string, string, map[string]*v1alpha1.SliceGatewayServiceType, *v1alpha1.TopologySpec) error); ok { + r1 = rf(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap, topology) } else { r1 = ret.Error(1) } diff --git a/service/slice_config_service.go b/service/slice_config_service.go index 539a82ef..1720f75b 100644 --- a/service/slice_config_service.go +++ b/service/slice_config_service.go @@ -204,7 +204,7 @@ func (s *SliceConfigService) ReconcileSliceConfig(ctx context.Context, req ctrl. } // Step 5: Create gateways with minimum specification - _, err = s.sgs.CreateMinimumWorkerSliceGateways(ctx, sliceConfig.Name, sliceConfig.Spec.Clusters, req.Namespace, ownershipLabel, clusterMap, sliceConfig.Spec.SliceSubnet, clusterCidr, sliceGwSvcTypeMap) + _, err = s.sgs.CreateMinimumWorkerSliceGateways(ctx, sliceConfig.Name, sliceConfig.Spec.Clusters, req.Namespace, ownershipLabel, clusterMap, sliceConfig.Spec.SliceSubnet, clusterCidr, sliceGwSvcTypeMap, sliceConfig.Spec.Topology) if err != nil { return ctrl.Result{}, err } diff --git a/service/slice_config_service_test.go b/service/slice_config_service_test.go index 1f317b5a..aacc6d3d 100644 --- a/service/slice_config_service_test.go +++ b/service/slice_config_service_test.go @@ -102,7 +102,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, 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, mock.Anything).Return(ctrl.Result{}, nil).Once() label := map[string]string{ "original-slice-name": sliceConfig.Name, } @@ -350,7 +350,7 @@ func SliceConfigErrorOnCreateWorkerSliceGateway(t *testing.T) { clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(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, 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, mock.Anything).Return(ctrl.Result{}, err1).Once() result, err2 := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} require.Error(t, err2) @@ -626,7 +626,7 @@ func SliceConfigErrorOnListingServiceExport(t *testing.T) { } clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(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, 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, mock.Anything).Return(ctrl.Result{}, nil).Once() label := map[string]string{ "original-slice-name": sliceConfig.Name, } @@ -667,7 +667,7 @@ func SliceConfigErrorOnCreateOrUpdateServiceImport(t *testing.T) { clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(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, 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, 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 1da5079f..33626f06 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -46,7 +46,8 @@ 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, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType) (ctrl.Result, error) + label map[string]string, clusterMap map[string]int, sliceSubnet string, clusterCidr string, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType, + topology *controllerv1alpha1.TopologySpec) (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 @@ -348,7 +349,8 @@ 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, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType) (ctrl.Result, error) { + sliceSubnet string, clusterCidr string, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType, + topology *controllerv1alpha1.TopologySpec) (ctrl.Result, error) { err := s.cleanupObsoleteGateways(ctx, namespace, label, clusterNames, clusterMap) if err != nil { @@ -358,7 +360,8 @@ func (s *WorkerSliceGatewayService) CreateMinimumWorkerSliceGateways(ctx context return ctrl.Result{}, nil } - _, err = s.createMinimumGatewaysIfNotExists(ctx, sliceName, clusterNames, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) + desiredEdges := ResolveTopologyEdges(clusterNames, topology) + _, err = s.createMinimumGatewaysIfNotExists(ctx, sliceName, desiredEdges, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) if err != nil { return ctrl.Result{}, err } @@ -435,43 +438,44 @@ func (s *WorkerSliceGatewayService) cleanupObsoleteGateways(ctx context.Context, return nil } -// createMinimumGatewaysIfNotExists is a helper function to create the gateways between worker clusters if not exists +// createMinimumGatewaysIfNotExists creates the gateway pairs for the desired topology edges if not present. func (s *WorkerSliceGatewayService) createMinimumGatewaysIfNotExists(ctx context.Context, sliceName string, - clusterNames []string, namespace string, ownerLabel map[string]string, clusterMap map[string]int, + desiredEdges []TopologyEdge, 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{} - found, err := util.GetResourceIfExist(ctx, client.ObjectKey{Name: clusterName, Namespace: namespace}, &cluster) - if !found || err != nil { - return ctrl.Result{}, err - } - clusterMapping[clusterName] = &cluster - } - for i := 0; i < noClusters; i++ { - for j := i + 1; j < noClusters; j++ { - 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) - // determine the gateway svc parameters - sliceGwSvcType := defaultSliceGatewayServiceType - gwSvcProtocol := defaultSliceGatewayServiceProtocol - if val, exists := sliceGwSvcTypeMap[sourceCluster.Name]; exists { - sliceGwSvcType = val.Type - gwSvcProtocol = val.Protocol + for _, edge := range desiredEdges { + for _, clusterName := range []string{edge.ServerCluster, edge.ClientCluster} { + if clusterMapping[clusterName] != nil { + continue } - 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 { + cluster := controllerv1alpha1.Cluster{} + found, err := util.GetResourceIfExist(ctx, client.ObjectKey{Name: clusterName, Namespace: namespace}, &cluster) + if !found || err != nil { return ctrl.Result{}, err } + clusterMapping[clusterName] = &cluster + } + } + for _, edge := range desiredEdges { + sourceCluster, destinationCluster := clusterMapping[edge.ServerCluster], clusterMapping[edge.ClientCluster] + gatewayNumber := s.calculateGatewayNumber(clusterMap[sourceCluster.Name], clusterMap[destinationCluster.Name]) + 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 + } + 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 } } return ctrl.Result{}, nil - } // createMinimumGateWayPairIfNotExists is a function to create the pair of gatways between 2 clusters if not exists diff --git a/service/worker_slice_gateway_service_test.go b/service/worker_slice_gateway_service_test.go index f09906b8..d314c921 100644 --- a/service/worker_slice_gateway_service_test.go +++ b/service/worker_slice_gateway_service_test.go @@ -320,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", nil) + result, err := workerSliceGatewayService.CreateMinimumWorkerSliceGateways(ctx, "red", clusterNames, requestObj.Namespace, label, clusterMap, "10.10.10.10/16", "/16", nil, nil) expectedResult := ctrl.Result{} require.NoError(t, nil) require.Equal(t, result, expectedResult) @@ -407,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", nil) + result, err := workerSliceGatewayService.CreateMinimumWorkerSliceGateways(ctx, "red", clusterNames, requestObj.Namespace, label, clusterMap, "10.10.10.10/16", "/16", nil, nil) expectedResult := ctrl.Result{} require.NoError(t, nil) require.Equal(t, result, expectedResult) From 084da7a982bd0e0d3cdc5d96d312c630023fec56 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Sun, 12 Jul 2026 22:04:03 +0530 Subject: [PATCH 06/21] test: hub-and-spoke create skips spoke-to-spoke edges Signed-off-by: Shreesha001 --- service/worker_slice_gateway_service_test.go | 58 +++++++++++++++++--- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/service/worker_slice_gateway_service_test.go b/service/worker_slice_gateway_service_test.go index d314c921..cae8fcde 100644 --- a/service/worker_slice_gateway_service_test.go +++ b/service/worker_slice_gateway_service_test.go @@ -57,15 +57,16 @@ func TestWorkerSliceGatewaySuite(t *testing.T) { } var WorkerSliceGatewayTestbed = map[string]func(*testing.T){ - "TestWorkerSliceGatewayReconciliation_Success": testWorkerSliceGatewayReconciliationSuccess, - "TestWorkerSliceGatewayReconciliation_IfSliceConfigNotFound": testWorkerSliceGatewayReconciliationIfSliceConfigNotFound, - "TestWorkerSliceGatewayReconciliation_IfGatewayNotFound": testWorkerSliceGatewayReconciliationIfGatewayNotFound, - "TestWorkerSliceGatewayReconciliation_Delete": testWorkerSliceGatewayReconciliationDelete, - "TestWorkerSliceGatewayReconciliation_DeleteForcefully": testWorkerSliceGatewayReconciliationDeleteForcefully, - "TestCreateMinimumWorkerSliceGateways_IfAlreadyExists": testCreateMinimumWorkerSliceGatewaysAlreadyExists, - "TestCreateMinimumWorkerSliceGateways_IfNotExists": testCreateMinimumWorkerSliceGatewaysNotExists, - "TestDeleteWorkerSliceGatewaysByLabel_IfExists": testDeleteWorkerSliceGatewaysByLabelExists, - "TestNodeIpReconciliationOfWorkerSliceGateways_IfExists": testNodeIpReconciliationOfWorkerSliceGatewaysExists, + "TestWorkerSliceGatewayReconciliation_Success": testWorkerSliceGatewayReconciliationSuccess, + "TestWorkerSliceGatewayReconciliation_IfSliceConfigNotFound": testWorkerSliceGatewayReconciliationIfSliceConfigNotFound, + "TestWorkerSliceGatewayReconciliation_IfGatewayNotFound": testWorkerSliceGatewayReconciliationIfGatewayNotFound, + "TestWorkerSliceGatewayReconciliation_Delete": testWorkerSliceGatewayReconciliationDelete, + "TestWorkerSliceGatewayReconciliation_DeleteForcefully": testWorkerSliceGatewayReconciliationDeleteForcefully, + "TestCreateMinimumWorkerSliceGateways_IfAlreadyExists": testCreateMinimumWorkerSliceGatewaysAlreadyExists, + "TestCreateMinimumWorkerSliceGateways_HubAndSpokeSkipsSpokeToSpoke": testCreateMinimumWorkerSliceGatewaysHubAndSpokeSkipsSpokeToSpoke, + "TestCreateMinimumWorkerSliceGateways_IfNotExists": testCreateMinimumWorkerSliceGatewaysNotExists, + "TestDeleteWorkerSliceGatewaysByLabel_IfExists": testDeleteWorkerSliceGatewaysByLabelExists, + "TestNodeIpReconciliationOfWorkerSliceGateways_IfExists": testNodeIpReconciliationOfWorkerSliceGatewaysExists, } func testWorkerSliceGatewayReconciliationSuccess(t *testing.T) { @@ -329,6 +330,45 @@ func testCreateMinimumWorkerSliceGatewaysAlreadyExists(t *testing.T) { mMock.AssertExpectations(t) } +// testCreateMinimumWorkerSliceGatewaysHubAndSpokeSkipsSpokeToSpoke verifies that +// for a HubAndSpoke slice with hub=cluster-1 and three clusters, only the two +// hub<->spoke edges are processed (cluster-1<->cluster-2, cluster-1<->cluster-3) +// and the spoke<->spoke pair (cluster-2<->cluster-3) is never created. The proof +// is in the mock call counts: exactly 3 cluster fetches and exactly 4 gateway +// existence checks (2 pairs x server+client); a spoke<->spoke edge would add two +// more gateway checks and exceed these expectations. +func testCreateMinimumWorkerSliceGatewaysHubAndSpokeSkipsSpokeToSpoke(t *testing.T) { + _, _, _, workerSliceGatewayService, requestObj, clientMock, _, ctx, mMock := setupWorkerSliceGatewayTest("slice_gateway", "namespace") + label := map[string]string{} + clusterNames := []string{"cluster-1", "cluster-2", "cluster-3"} + clusterMap := map[string]int{ + "cluster-1": 1, + "cluster-2": 2, + "cluster-3": 3, + } + topology := &controllerv1alpha1.TopologySpec{ + Mode: controllerv1alpha1.TopologyModeHubAndSpoke, + Hubs: []string{"cluster-1"}, + } + mMock.On("WithProject", mock.AnythingOfType("string")).Return(&metrics.MetricRecorder{}).Once() + // cleanup pass: no existing gateways to remove + pairWorkerSliceGateway := &workerv1alpha1.WorkerSliceGatewayList{} + clientMock.On("List", ctx, pairWorkerSliceGateway, mock.Anything, client.InNamespace(requestObj.Namespace)).Return(nil).Once() + // create pass: clusters participating in the two desired edges are fetched + cluster := &controllerv1alpha1.Cluster{} + clientMock.On("Get", ctx, mock.AnythingOfType("types.NamespacedName"), cluster).Return(nil).Times(3) + // gateway existence checks: exactly 2 hub<->spoke pairs (server+client each), + // all found -> nothing created. A spoke<->spoke pair would exceed 4 checks. + gateway := &workerv1alpha1.WorkerSliceGateway{} + clientMock.On("Get", ctx, mock.AnythingOfType("types.NamespacedName"), gateway).Return(nil).Times(4) + + result, err := workerSliceGatewayService.CreateMinimumWorkerSliceGateways(ctx, "red", clusterNames, requestObj.Namespace, label, clusterMap, "10.10.10.10/16", "/16", nil, topology) + require.Equal(t, ctrl.Result{}, result) + require.Nil(t, err) + clientMock.AssertExpectations(t) + mMock.AssertExpectations(t) +} + func testCreateMinimumWorkerSliceGatewaysNotExists(t *testing.T) { _, _, jobMock, workerSliceGatewayService, requestObj, clientMock, _, ctx, mMock := setupWorkerSliceGatewayTest("slice_gateway", "namespace") label := map[string]string{ From 6879152ee75196b964cf1877126f90408142d03f Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Mon, 13 Jul 2026 17:45:23 +0530 Subject: [PATCH 07/21] feat: remove no-longer-desired edges on topology change (cleanup gating) Signed-off-by: Shreesha001 --- service/topology_resolver.go | 29 +++++++ service/topology_resolver_test.go | 19 +++++ service/worker_slice_gateway_service.go | 11 ++- service/worker_slice_gateway_service_test.go | 84 +++++++++++++++++--- 4 files changed, 129 insertions(+), 14 deletions(-) diff --git a/service/topology_resolver.go b/service/topology_resolver.go index 225bbd82..01cd674b 100644 --- a/service/topology_resolver.go +++ b/service/topology_resolver.go @@ -67,3 +67,32 @@ func ResolveTopologyEdges(clusters []string, topology *controllerv1alpha1.Topolo } return edges } + +// TopologyEdgeSet answers direction-insensitive membership questions about a +// set of desired edges: the two WorkerSliceGateway objects of a pair (the +// server side and the client side) belong to the same logical edge. +type TopologyEdgeSet struct { + members map[[2]string]bool +} + +// NewTopologyEdgeSet builds a TopologyEdgeSet from resolved edges. +func NewTopologyEdgeSet(edges []TopologyEdge) TopologyEdgeSet { + members := make(map[[2]string]bool, len(edges)) + for _, edge := range edges { + members[edgeKey(edge.ServerCluster, edge.ClientCluster)] = true + } + return TopologyEdgeSet{members: members} +} + +// Contains reports whether the given cluster pair, in either order, is a +// desired edge. +func (s TopologyEdgeSet) Contains(clusterA, clusterB string) bool { + return s.members[edgeKey(clusterA, clusterB)] +} + +func edgeKey(clusterA, clusterB string) [2]string { + if clusterA > clusterB { + clusterA, clusterB = clusterB, clusterA + } + return [2]string{clusterA, clusterB} +} diff --git a/service/topology_resolver_test.go b/service/topology_resolver_test.go index 27a648b9..4358a911 100644 --- a/service/topology_resolver_test.go +++ b/service/topology_resolver_test.go @@ -78,3 +78,22 @@ func TestResolveTopologyEdges(t *testing.T) { }) } } + +func TestTopologyEdgeSetContains(t *testing.T) { + // hub-and-spoke edges: hub=worker-1, spokes worker-2/worker-3 + set := NewTopologyEdgeSet(ResolveTopologyEdges( + []string{"worker-1", "worker-2", "worker-3"}, + &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"worker-1"}}, + )) + // desired hub<->spoke edges, both directions + if !set.Contains("worker-1", "worker-2") || !set.Contains("worker-2", "worker-1") { + t.Fatal("expected worker-1<->worker-2 to be a desired edge (either direction)") + } + if !set.Contains("worker-1", "worker-3") { + t.Fatal("expected worker-1<->worker-3 to be a desired edge") + } + // spoke<->spoke is NOT desired + if set.Contains("worker-2", "worker-3") || set.Contains("worker-3", "worker-2") { + t.Fatal("did not expect worker-2<->worker-3 (spoke-to-spoke) to be a desired edge") + } +} diff --git a/service/worker_slice_gateway_service.go b/service/worker_slice_gateway_service.go index 33626f06..bcee202b 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -352,7 +352,8 @@ func (s *WorkerSliceGatewayService) CreateMinimumWorkerSliceGateways(ctx context sliceSubnet string, clusterCidr string, sliceGwSvcTypeMap map[string]*controllerv1alpha1.SliceGatewayServiceType, topology *controllerv1alpha1.TopologySpec) (ctrl.Result, error) { - err := s.cleanupObsoleteGateways(ctx, namespace, label, clusterNames, clusterMap) + desiredEdges := ResolveTopologyEdges(clusterNames, topology) + err := s.cleanupObsoleteGateways(ctx, namespace, label, clusterNames, clusterMap, NewTopologyEdgeSet(desiredEdges)) if err != nil { return ctrl.Result{}, err } @@ -360,7 +361,6 @@ func (s *WorkerSliceGatewayService) CreateMinimumWorkerSliceGateways(ctx context return ctrl.Result{}, nil } - desiredEdges := ResolveTopologyEdges(clusterNames, topology) _, err = s.createMinimumGatewaysIfNotExists(ctx, sliceName, desiredEdges, namespace, label, clusterMap, sliceSubnet, clusterCidr, sliceGwSvcTypeMap) if err != nil { return ctrl.Result{}, err @@ -381,7 +381,7 @@ func (s *WorkerSliceGatewayService) ListWorkerSliceGateways(ctx context.Context, // cleanupObsoleteGateways is a function delete outdated gateways func (s *WorkerSliceGatewayService) cleanupObsoleteGateways(ctx context.Context, namespace string, ownerLabel map[string]string, - clusters []string, clusterMap map[string]int) error { + clusters []string, clusterMap map[string]int, desiredEdges TopologyEdgeSet) error { gateways, err := s.ListWorkerSliceGateways(ctx, ownerLabel, namespace) if err != nil { @@ -408,7 +408,10 @@ func (s *WorkerSliceGatewayService) cleanupObsoleteGateways(ctx context.Context, clusterSource := gateway.Spec.LocalGatewayConfig.ClusterName clusterDestination := gateway.Spec.RemoteGatewayConfig.ClusterName gatewayExpectedNumber := s.calculateGatewayNumber(clusterMap[clusterSource], clusterMap[clusterDestination]) - if !clusterExistMap[clusterSource] || !clusterExistMap[clusterDestination] || gatewayExpectedNumber != gateway.Spec.GatewayNumber { + // Delete a gateway when either cluster left the slice, its gateway number + // changed, or its edge is no longer in the desired topology (e.g. a + // spoke<->spoke link after a FullMesh->HubAndSpoke change). + if !clusterExistMap[clusterSource] || !clusterExistMap[clusterDestination] || gatewayExpectedNumber != gateway.Spec.GatewayNumber || !desiredEdges.Contains(clusterSource, clusterDestination) { err = util.DeleteResource(ctx, &gateway) if err != nil { //Register an event for worker slice gateway deletion failure diff --git a/service/worker_slice_gateway_service_test.go b/service/worker_slice_gateway_service_test.go index cae8fcde..d3ab85b2 100644 --- a/service/worker_slice_gateway_service_test.go +++ b/service/worker_slice_gateway_service_test.go @@ -57,16 +57,17 @@ func TestWorkerSliceGatewaySuite(t *testing.T) { } var WorkerSliceGatewayTestbed = map[string]func(*testing.T){ - "TestWorkerSliceGatewayReconciliation_Success": testWorkerSliceGatewayReconciliationSuccess, - "TestWorkerSliceGatewayReconciliation_IfSliceConfigNotFound": testWorkerSliceGatewayReconciliationIfSliceConfigNotFound, - "TestWorkerSliceGatewayReconciliation_IfGatewayNotFound": testWorkerSliceGatewayReconciliationIfGatewayNotFound, - "TestWorkerSliceGatewayReconciliation_Delete": testWorkerSliceGatewayReconciliationDelete, - "TestWorkerSliceGatewayReconciliation_DeleteForcefully": testWorkerSliceGatewayReconciliationDeleteForcefully, - "TestCreateMinimumWorkerSliceGateways_IfAlreadyExists": testCreateMinimumWorkerSliceGatewaysAlreadyExists, - "TestCreateMinimumWorkerSliceGateways_HubAndSpokeSkipsSpokeToSpoke": testCreateMinimumWorkerSliceGatewaysHubAndSpokeSkipsSpokeToSpoke, - "TestCreateMinimumWorkerSliceGateways_IfNotExists": testCreateMinimumWorkerSliceGatewaysNotExists, - "TestDeleteWorkerSliceGatewaysByLabel_IfExists": testDeleteWorkerSliceGatewaysByLabelExists, - "TestNodeIpReconciliationOfWorkerSliceGateways_IfExists": testNodeIpReconciliationOfWorkerSliceGatewaysExists, + "TestWorkerSliceGatewayReconciliation_Success": testWorkerSliceGatewayReconciliationSuccess, + "TestWorkerSliceGatewayReconciliation_IfSliceConfigNotFound": testWorkerSliceGatewayReconciliationIfSliceConfigNotFound, + "TestWorkerSliceGatewayReconciliation_IfGatewayNotFound": testWorkerSliceGatewayReconciliationIfGatewayNotFound, + "TestWorkerSliceGatewayReconciliation_Delete": testWorkerSliceGatewayReconciliationDelete, + "TestWorkerSliceGatewayReconciliation_DeleteForcefully": testWorkerSliceGatewayReconciliationDeleteForcefully, + "TestCreateMinimumWorkerSliceGateways_IfAlreadyExists": testCreateMinimumWorkerSliceGatewaysAlreadyExists, + "TestCreateMinimumWorkerSliceGateways_HubAndSpokeSkipsSpokeToSpoke": testCreateMinimumWorkerSliceGatewaysHubAndSpokeSkipsSpokeToSpoke, + "TestCreateMinimumWorkerSliceGateways_HubAndSpokeCleansUpSpokeToSpoke": testCreateMinimumWorkerSliceGatewaysHubAndSpokeCleansUpSpokeToSpoke, + "TestCreateMinimumWorkerSliceGateways_IfNotExists": testCreateMinimumWorkerSliceGatewaysNotExists, + "TestDeleteWorkerSliceGatewaysByLabel_IfExists": testDeleteWorkerSliceGatewaysByLabelExists, + "TestNodeIpReconciliationOfWorkerSliceGateways_IfExists": testNodeIpReconciliationOfWorkerSliceGatewaysExists, } func testWorkerSliceGatewayReconciliationSuccess(t *testing.T) { @@ -369,6 +370,69 @@ func testCreateMinimumWorkerSliceGatewaysHubAndSpokeSkipsSpokeToSpoke(t *testing mMock.AssertExpectations(t) } +// testCreateMinimumWorkerSliceGatewaysHubAndSpokeCleansUpSpokeToSpoke verifies +// that when a slice already has a spoke<->spoke gateway pair (e.g. left over from +// a FullMesh->HubAndSpoke change), cleanup deletes it purely because its edge is +// no longer in the desired hub-and-spoke set. The stale pair is given the CORRECT +// gateway number and both clusters are still slice members, so the ONLY reason it +// gets removed is the topology edge check. +func testCreateMinimumWorkerSliceGatewaysHubAndSpokeCleansUpSpokeToSpoke(t *testing.T) { + _, _, _, workerSliceGatewayService, requestObj, clientMock, _, ctx, mMock := setupWorkerSliceGatewayTest("slice_gateway", "namespace") + label := map[string]string{} + clusterNames := []string{"cluster-1", "cluster-2", "cluster-3"} + clusterMap := map[string]int{ + "cluster-1": 1, + "cluster-2": 2, + "cluster-3": 3, + } + topology := &controllerv1alpha1.TopologySpec{ + Mode: controllerv1alpha1.TopologyModeHubAndSpoke, + Hubs: []string{"cluster-1"}, + } + mMock.On("WithProject", mock.AnythingOfType("string")).Return(&metrics.MetricRecorder{}).Once() + // existing spoke<->spoke pair (cluster-2 <-> cluster-3) with the CORRECT gateway + // number; both clusters are still members, so it survives the membership and + // number checks and is removed only because its edge is not desired. + spokeToSpokeNumber := ((3-1)*(3-2))/2 + 2 // calculateGatewayNumber(2, 3) = 3 + pairWorkerSliceGateway := &workerv1alpha1.WorkerSliceGatewayList{} + clientMock.On("List", ctx, pairWorkerSliceGateway, mock.Anything, client.InNamespace(requestObj.Namespace)).Return(nil).Run(func(args mock.Arguments) { + arg := args.Get(1).(*workerv1alpha1.WorkerSliceGatewayList) + arg.Items = []workerv1alpha1.WorkerSliceGateway{ + { + Spec: workerv1alpha1.WorkerSliceGatewaySpec{ + LocalGatewayConfig: workerv1alpha1.SliceGatewayConfig{ClusterName: "cluster-2"}, + RemoteGatewayConfig: workerv1alpha1.SliceGatewayConfig{ClusterName: "cluster-3"}, + GatewayNumber: spokeToSpokeNumber, + }, + }, + { + Spec: workerv1alpha1.WorkerSliceGatewaySpec{ + LocalGatewayConfig: workerv1alpha1.SliceGatewayConfig{ClusterName: "cluster-3"}, + RemoteGatewayConfig: workerv1alpha1.SliceGatewayConfig{ClusterName: "cluster-2"}, + GatewayNumber: spokeToSpokeNumber, + }, + }, + } + }).Once() + clientMock.On("Delete", ctx, mock.Anything).Return(nil).Twice() + clientMock.On("Create", ctx, mock.AnythingOfType("*v1.Event")).Return(nil).Once() + 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() + // create pass for the two desired hub<->spoke edges: clusters fetched, gateways + // already exist -> nothing created. + cluster := &controllerv1alpha1.Cluster{} + clientMock.On("Get", ctx, mock.AnythingOfType("types.NamespacedName"), cluster).Return(nil).Times(3) + gateway := &workerv1alpha1.WorkerSliceGateway{} + clientMock.On("Get", ctx, mock.AnythingOfType("types.NamespacedName"), gateway).Return(nil).Times(4) + + result, err := workerSliceGatewayService.CreateMinimumWorkerSliceGateways(ctx, "red", clusterNames, requestObj.Namespace, label, clusterMap, "10.10.10.10/16", "/16", nil, topology) + require.Equal(t, ctrl.Result{}, result) + require.Nil(t, err) + clientMock.AssertExpectations(t) + mMock.AssertExpectations(t) +} + func testCreateMinimumWorkerSliceGatewaysNotExists(t *testing.T) { _, _, jobMock, workerSliceGatewayService, requestObj, clientMock, _, ctx, mMock := setupWorkerSliceGatewayTest("slice_gateway", "namespace") label := map[string]string{ From 91bf1c74cf44e0b96f25f00f6c2b1cc83b7fe6a3 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Wed, 15 Jul 2026 23:28:13 +0530 Subject: [PATCH 08/21] feat: add gateway connectivity and slice topology status fields Signed-off-by: Shreesha001 --- apis/controller/v1alpha1/sliceconfig_types.go | 20 ++++++ .../v1alpha1/zz_generated.deepcopy.go | 8 +++ .../v1alpha1/workerslicegateway_types.go | 23 +++++++ apis/worker/v1alpha1/zz_generated.deepcopy.go | 6 +- .../controller.kubeslice.io_sliceconfigs.yaml | 63 ++++++++++++++++++- ...rker.kubeslice.io_workerslicegateways.yaml | 18 +++++- 6 files changed, 135 insertions(+), 3 deletions(-) diff --git a/apis/controller/v1alpha1/sliceconfig_types.go b/apis/controller/v1alpha1/sliceconfig_types.go index 636befdc..9fc7a28c 100644 --- a/apis/controller/v1alpha1/sliceconfig_types.go +++ b/apis/controller/v1alpha1/sliceconfig_types.go @@ -220,8 +220,28 @@ type KubesliceEvent struct { } // SliceConfigStatus defines the observed state of SliceConfig +// Slice status condition types and reasons for topology convergence. +const ( + // SliceConditionTypeTopologyConverged reports whether every desired gateway + // link of the slice is Connected. + SliceConditionTypeTopologyConverged = "TopologyConverged" + // SliceReasonAllEdgesReady is set when all gateway links are Connected. + SliceReasonAllEdgesReady = "AllEdgesReady" + // SliceReasonEdgesNotReady is set when one or more gateway links are not Connected. + SliceReasonEdgesNotReady = "EdgesNotReady" + // SliceReasonNoGatewaysRequired is set when the slice needs no gateway links + // (single cluster or no-network mode) and is therefore trivially converged. + SliceReasonNoGatewaysRequired = "NoGatewaysRequired" +) + type SliceConfigStatus struct { KubesliceEvents []KubesliceEvent `json:"kubesliceEvents,omitempty"` + // Conditions represent the latest available observations of the slice's + // topology state (e.g. TopologyConverged). + //+optional + //+listType=map + //+listMapKey=type + Conditions []metav1.Condition `json:"conditions,omitempty"` } //+kubebuilder:object:root=true diff --git a/apis/controller/v1alpha1/zz_generated.deepcopy.go b/apis/controller/v1alpha1/zz_generated.deepcopy.go index 9a6d13e2..c40b9f8e 100644 --- a/apis/controller/v1alpha1/zz_generated.deepcopy.go +++ b/apis/controller/v1alpha1/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ limitations under the License. package v1alpha1 import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -791,6 +792,13 @@ func (in *SliceConfigStatus) DeepCopyInto(out *SliceConfigStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SliceConfigStatus. diff --git a/apis/worker/v1alpha1/workerslicegateway_types.go b/apis/worker/v1alpha1/workerslicegateway_types.go index 143381e4..2ce7d47b 100644 --- a/apis/worker/v1alpha1/workerslicegateway_types.go +++ b/apis/worker/v1alpha1/workerslicegateway_types.go @@ -61,9 +61,32 @@ type GatewayCredentials struct { } // WorkerSliceGatewayStatus defines the observed state of WorkerSliceGateway +// Gateway connection states reported by the worker on WorkerSliceGatewayStatus. +const ( + // GatewayConnectionStateConnected means the gateway tunnel is up (at least + // one HA gateway pod reports its tunnel established). + GatewayConnectionStateConnected = "Connected" + // GatewayConnectionStateNotConnected means the tunnel is down (all gateway + // pods report their tunnel not established). + GatewayConnectionStateNotConnected = "NotConnected" + // GatewayConnectionStatePending means no connectivity has been reported yet + // (e.g. the gateway was just created). An empty ConnectionState is treated + // as Pending by the controller-side aggregation. + GatewayConnectionStatePending = "Pending" +) + type WorkerSliceGatewayStatus struct { GatewayNumber int `json:"gatewayNumber,omitempty"` ClusterInsertionIndex int `json:"clusterInsertionIndex,omitempty"` + // ConnectionState is the connectivity state of this gateway link as reported + // by the worker: Connected, NotConnected or Pending. Empty means Pending. + ConnectionState string `json:"connectionState,omitempty"` + // LastTransitionTime is the time ConnectionState last changed. + LastTransitionTime *metav1.Time `json:"lastTransitionTime,omitempty"` + // Reason is a short, machine-readable reason for the current ConnectionState. + Reason string `json:"reason,omitempty"` + // Message is a human-readable description of the current ConnectionState. + Message string `json:"message,omitempty"` } //+kubebuilder:object:root=true diff --git a/apis/worker/v1alpha1/zz_generated.deepcopy.go b/apis/worker/v1alpha1/zz_generated.deepcopy.go index 578ce988..8d9fc229 100644 --- a/apis/worker/v1alpha1/zz_generated.deepcopy.go +++ b/apis/worker/v1alpha1/zz_generated.deepcopy.go @@ -497,7 +497,7 @@ func (in *WorkerSliceGateway) DeepCopyInto(out *WorkerSliceGateway) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkerSliceGateway. @@ -586,6 +586,10 @@ func (in *WorkerSliceGatewaySpec) DeepCopy() *WorkerSliceGatewaySpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *WorkerSliceGatewayStatus) DeepCopyInto(out *WorkerSliceGatewayStatus) { *out = *in + if in.LastTransitionTime != nil { + in, out := &in.LastTransitionTime, &out.LastTransitionTime + *out = (*in).DeepCopy() + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkerSliceGatewayStatus. diff --git a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml index f5591697..7187c949 100644 --- a/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml +++ b/config/crd/bases/controller.kubeslice.io_sliceconfigs.yaml @@ -266,8 +266,69 @@ spec: - maxClusters type: object status: - description: SliceConfigStatus defines the observed state of SliceConfig properties: + conditions: + description: |- + Conditions represent the latest available observations of the slice's + topology state (e.g. TopologyConverged). + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map kubesliceEvents: items: properties: diff --git a/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml b/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml index 8f47678e..400035c0 100644 --- a/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml +++ b/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml @@ -127,12 +127,28 @@ spec: type: string type: object status: - description: WorkerSliceGatewayStatus defines the observed state of WorkerSliceGateway properties: clusterInsertionIndex: type: integer + connectionState: + description: |- + ConnectionState is the connectivity state of this gateway link as reported + by the worker: Connected, NotConnected or Pending. Empty means Pending. + type: string gatewayNumber: type: integer + lastTransitionTime: + description: LastTransitionTime is the time ConnectionState last changed. + format: date-time + type: string + message: + description: Message is a human-readable description of the current + ConnectionState. + type: string + reason: + description: Reason is a short, machine-readable reason for the current + ConnectionState. + type: string type: object type: object served: true From 3c64e17ad6372e3b1cfdb67db9b07710ed627996 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Wed, 15 Jul 2026 23:28:13 +0530 Subject: [PATCH 09/21] feat: aggregate gateway connectivity into TopologyConverged slice condition Signed-off-by: Shreesha001 --- .../controller/sliceconfig_controller.go | 19 +++ service/slice_config_service.go | 38 ++++++ service/slice_config_service_test.go | 25 ++++ service/topology_status.go | 85 +++++++++++++ service/topology_status_test.go | 120 ++++++++++++++++++ 5 files changed, 287 insertions(+) create mode 100644 service/topology_status.go create mode 100644 service/topology_status_test.go diff --git a/controllers/controller/sliceconfig_controller.go b/controllers/controller/sliceconfig_controller.go index 99b4be95..c658c1e2 100644 --- a/controllers/controller/sliceconfig_controller.go +++ b/controllers/controller/sliceconfig_controller.go @@ -23,12 +23,15 @@ import ( "go.uber.org/zap" controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" + workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" "github.com/kubeslice/kubeslice-controller/service" "github.com/kubeslice/kubeslice-controller/util" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" ) // SliceConfigReconciler reconciles a SliceConfig object @@ -46,9 +49,25 @@ func (r *SliceConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) return r.SliceConfigService.ReconcileSliceConfig(kubeSliceCtx, req) } +// sliceConfigForGateway maps a WorkerSliceGateway to a reconcile request for the +// SliceConfig that owns it, so a change in a gateway's connectivity status +// re-triggers aggregation of the slice's TopologyConverged condition. The +// gateway carries its slice name in spec.SliceName and lives in the same +// (project) namespace as the SliceConfig. +func (r *SliceConfigReconciler) sliceConfigForGateway(ctx context.Context, obj client.Object) []ctrl.Request { + gateway, ok := obj.(*workerv1alpha1.WorkerSliceGateway) + if !ok || gateway.Spec.SliceName == "" { + return nil + } + return []ctrl.Request{ + {NamespacedName: types.NamespacedName{Name: gateway.Spec.SliceName, Namespace: gateway.Namespace}}, + } +} + // SetupWithManager sets up the controller with the Manager. func (r *SliceConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&controllerv1alpha1.SliceConfig{}). + Watches(&workerv1alpha1.WorkerSliceGateway{}, handler.EnqueueRequestsFromMapFunc(r.sliceConfigForGateway)). Complete(r) } diff --git a/service/slice_config_service.go b/service/slice_config_service.go index 1720f75b..e6316168 100644 --- a/service/slice_config_service.go +++ b/service/slice_config_service.go @@ -28,7 +28,9 @@ import ( "github.com/kubeslice/kubeslice-controller/events" "github.com/kubeslice/kubeslice-controller/util" corev1 "k8s.io/api/core/v1" + apimeta "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -238,9 +240,45 @@ func (s *SliceConfigService) ReconcileSliceConfig(ctx context.Context, req ctrl. } } + // Step 9: Aggregate per-gateway connectivity into the slice's TopologyConverged condition. + if err := s.reconcileTopologyStatus(ctx, sliceConfig, req.Namespace, ownershipLabel); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } +// reconcileTopologyStatus lists the slice's WorkerSliceGateway objects, aggregates +// their connectivity into the TopologyConverged condition on SliceConfig.status, +// and persists it only when the condition changed (so LastTransitionTime and the +// status subresource are not churned on every reconcile). +func (s *SliceConfigService) reconcileTopologyStatus(ctx context.Context, sliceConfig *v1alpha1.SliceConfig, namespace string, ownershipLabel map[string]string) error { + gateways, err := s.sgs.ListWorkerSliceGateways(ctx, ownershipLabel, namespace) + if err != nil { + return err + } + condition := buildTopologyConvergedCondition(gateways, sliceConfig.Generation) + // The SliceConfig has already been mutated earlier in this reconcile (finalizer, + // labels), and the WorkerSliceGateway watch can drive concurrent reconciles, so + // the in-memory copy may be stale. Re-fetch the latest object and retry on a + // write conflict rather than failing the whole reconcile. + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + latest := &v1alpha1.SliceConfig{} + found, err := util.GetResourceIfExist(ctx, client.ObjectKey{Name: sliceConfig.Name, Namespace: namespace}, latest) + if err != nil { + return err + } + if !found { + return nil + } + condition.ObservedGeneration = latest.Generation + if !apimeta.SetStatusCondition(&latest.Status.Conditions, condition) { + return nil + } + return util.UpdateStatus(ctx, latest) + }) +} + // checkForProjectNamespace is a function to check the namespace is in proper format func (s *SliceConfigService) checkForProjectNamespace(namespace *corev1.Namespace) bool { return namespace.Labels[util.LabelName] == fmt.Sprintf(util.LabelValue, "Project", namespace.Name) diff --git a/service/slice_config_service_test.go b/service/slice_config_service_test.go index aacc6d3d..cceb0900 100644 --- a/service/slice_config_service_test.go +++ b/service/slice_config_service_test.go @@ -30,6 +30,7 @@ import ( "github.com/dailymotion/allure-go" controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" + workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" ossEvents "github.com/kubeslice/kubeslice-controller/events" "github.com/kubeslice/kubeslice-controller/service/mocks" @@ -80,6 +81,26 @@ var SliceConfigTestBed = map[string]func(*testing.T){ "SliceConfig_ErrorOnCreateOrUpdateServiceImport": SliceConfigErrorOnCreateOrUpdateServiceImport, } +// fakeStatusWriter is a minimal client.SubResourceWriter for tests that exercise +// status subresource updates. It records the last object passed to Update so +// callers may assert on the written status. +type fakeStatusWriter struct { + updated client.Object +} + +func (f *fakeStatusWriter) Create(ctx context.Context, obj client.Object, subResource client.Object, opts ...client.SubResourceCreateOption) error { + return nil +} + +func (f *fakeStatusWriter) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + f.updated = obj + return nil +} + +func (f *fakeStatusWriter) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { + return nil +} + func SliceConfigReconciliationCompleteHappyCase(t *testing.T) { workerSliceGatewayMock, workerSliceConfigMock, _, workerServiceImportMock, _, clientMock, sliceConfig, ctx, sliceConfigService, requestObj, mMock := setupSliceConfigTest("slice_config", "namespace") mMock.On("WithProject", mock.AnythingOfType("string")).Return(&metrics.MetricRecorder{}).Once() @@ -119,6 +140,10 @@ func SliceConfigReconciliationCompleteHappyCase(t *testing.T) { } }).Once() workerServiceImportMock.On("CreateMinimalWorkerServiceImport", ctx, sliceConfig.Spec.Clusters, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + // Step 9 (topology status aggregation): no gateways -> TopologyConverged is set + // for the first time, so the condition changes and the status is written. + workerSliceGatewayMock.On("ListWorkerSliceGateways", ctx, mock.Anything, requestObj.Namespace).Return([]workerv1alpha1.WorkerSliceGateway{}, nil).Once() + clientMock.On("Status").Return(&fakeStatusWriter{}) result, err := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} require.NoError(t, nil) diff --git a/service/topology_status.go b/service/topology_status.go new file mode 100644 index 00000000..58e8c68a --- /dev/null +++ b/service/topology_status.go @@ -0,0 +1,85 @@ +/* + * Copyright (c) 2026 Avesha, Inc. All rights reserved. # # SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package service + +import ( + "fmt" + + controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" + workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// buildTopologyConvergedCondition aggregates the connectivity of a slice's +// WorkerSliceGateway objects into a single TopologyConverged condition. +// +// A slice is converged (ConditionTrue) when every gateway link reports +// ConnectionState == Connected, or when the slice has no gateway links at all. +// Otherwise it is ConditionFalse and the message identifies how many links are +// connected and names the first not-connected link (chosen deterministically by +// gateway name so the message is stable across reconciles). An empty +// ConnectionState is treated as Pending. +// +// LastTransitionTime is intentionally left unset: callers apply this condition +// via apimachinery meta.SetStatusCondition, which stamps the transition time +// only when the status actually changes. +func buildTopologyConvergedCondition(gateways []workerv1alpha1.WorkerSliceGateway, observedGeneration int64) metav1.Condition { + cond := metav1.Condition{ + Type: controllerv1alpha1.SliceConditionTypeTopologyConverged, + ObservedGeneration: observedGeneration, + } + + total := len(gateways) + if total == 0 { + cond.Status = metav1.ConditionTrue + cond.Reason = controllerv1alpha1.SliceReasonNoGatewaysRequired + cond.Message = "slice requires no gateway links" + return cond + } + + ready := 0 + notReadyFound := false + firstNotReadyName := "" + firstNotReadyState := "" + for i := range gateways { + state := gateways[i].Status.ConnectionState + if state == workerv1alpha1.GatewayConnectionStateConnected { + ready++ + continue + } + if state == "" { + state = workerv1alpha1.GatewayConnectionStatePending + } + if !notReadyFound || gateways[i].Name < firstNotReadyName { + notReadyFound = true + firstNotReadyName = gateways[i].Name + firstNotReadyState = state + } + } + + if ready == total { + cond.Status = metav1.ConditionTrue + cond.Reason = controllerv1alpha1.SliceReasonAllEdgesReady + cond.Message = fmt.Sprintf("all %d gateway links connected", total) + return cond + } + + cond.Status = metav1.ConditionFalse + cond.Reason = controllerv1alpha1.SliceReasonEdgesNotReady + cond.Message = fmt.Sprintf("%d/%d gateway links connected; %s is %s", ready, total, firstNotReadyName, firstNotReadyState) + return cond +} diff --git a/service/topology_status_test.go b/service/topology_status_test.go new file mode 100644 index 00000000..e4bae043 --- /dev/null +++ b/service/topology_status_test.go @@ -0,0 +1,120 @@ +/* + * Copyright (c) 2026 Avesha, Inc. All rights reserved. # # SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package service + +import ( + "strings" + "testing" + + controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" + workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func gw(name, state string) workerv1alpha1.WorkerSliceGateway { + g := workerv1alpha1.WorkerSliceGateway{} + g.Name = name + g.Status.ConnectionState = state + return g +} + +func TestBuildTopologyConvergedCondition(t *testing.T) { + cases := []struct { + name string + gateways []workerv1alpha1.WorkerSliceGateway + wantStatus metav1.ConditionStatus + wantReason string + msgContains []string + msgNotContains []string + }{ + { + name: "no gateways is trivially converged", + gateways: nil, + wantStatus: metav1.ConditionTrue, + wantReason: controllerv1alpha1.SliceReasonNoGatewaysRequired, + msgContains: []string{"no gateway links"}, + }, + { + name: "all connected is converged", + gateways: []workerv1alpha1.WorkerSliceGateway{ + gw("slice-hub-spoke1", workerv1alpha1.GatewayConnectionStateConnected), + gw("slice-spoke1-hub", workerv1alpha1.GatewayConnectionStateConnected), + }, + wantStatus: metav1.ConditionTrue, + wantReason: controllerv1alpha1.SliceReasonAllEdgesReady, + msgContains: []string{"all", "2"}, + }, + { + name: "one not connected is not converged and is named", + gateways: []workerv1alpha1.WorkerSliceGateway{ + gw("slice-hub-spoke1", workerv1alpha1.GatewayConnectionStateConnected), + gw("slice-hub-spoke2", workerv1alpha1.GatewayConnectionStateNotConnected), + }, + wantStatus: metav1.ConditionFalse, + wantReason: controllerv1alpha1.SliceReasonEdgesNotReady, + msgContains: []string{"1/2", "slice-hub-spoke2", "NotConnected"}, + }, + { + name: "empty connection state is reported as Pending", + gateways: []workerv1alpha1.WorkerSliceGateway{ + gw("slice-hub-spoke1", ""), + }, + wantStatus: metav1.ConditionFalse, + wantReason: controllerv1alpha1.SliceReasonEdgesNotReady, + msgContains: []string{"0/1", "Pending"}, + }, + { + name: "first not-ready is chosen deterministically by name", + gateways: []workerv1alpha1.WorkerSliceGateway{ + gw("slice-z", workerv1alpha1.GatewayConnectionStateNotConnected), + gw("slice-a", workerv1alpha1.GatewayConnectionStateNotConnected), + gw("slice-m", workerv1alpha1.GatewayConnectionStateConnected), + }, + wantStatus: metav1.ConditionFalse, + wantReason: controllerv1alpha1.SliceReasonEdgesNotReady, + msgContains: []string{"1/3", "slice-a"}, + msgNotContains: []string{"slice-z"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cond := buildTopologyConvergedCondition(tc.gateways, 7) + if cond.Type != controllerv1alpha1.SliceConditionTypeTopologyConverged { + t.Fatalf("type = %q, want %q", cond.Type, controllerv1alpha1.SliceConditionTypeTopologyConverged) + } + if cond.Status != tc.wantStatus { + t.Fatalf("status = %q, want %q (msg=%q)", cond.Status, tc.wantStatus, cond.Message) + } + if cond.Reason != tc.wantReason { + t.Fatalf("reason = %q, want %q", cond.Reason, tc.wantReason) + } + if cond.ObservedGeneration != 7 { + t.Fatalf("observedGeneration = %d, want 7", cond.ObservedGeneration) + } + for _, sub := range tc.msgContains { + if !strings.Contains(cond.Message, sub) { + t.Fatalf("message %q does not contain %q", cond.Message, sub) + } + } + for _, sub := range tc.msgNotContains { + if strings.Contains(cond.Message, sub) { + t.Fatalf("message %q should not contain %q", cond.Message, sub) + } + } + }) + } +} From d9a589c72468b45ddd63cefe93075e08bf046c3e Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Thu, 16 Jul 2026 22:29:59 +0530 Subject: [PATCH 10/21] feat: mark spoke gateways to route entire slice subnet via hub Signed-off-by: Shreesha001 --- apis/worker/v1alpha1/workerslicegateway_types.go | 6 ++++++ .../bases/worker.kubeslice.io_workerslicegateways.yaml | 8 ++++++++ service/topology_resolver.go | 6 +++++- service/topology_resolver_test.go | 8 ++++---- service/worker_slice_gateway_service.go | 8 ++++++-- 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/apis/worker/v1alpha1/workerslicegateway_types.go b/apis/worker/v1alpha1/workerslicegateway_types.go index 143381e4..3b88762b 100644 --- a/apis/worker/v1alpha1/workerslicegateway_types.go +++ b/apis/worker/v1alpha1/workerslicegateway_types.go @@ -41,6 +41,12 @@ type WorkerSliceGatewaySpec struct { LocalGatewayConfig SliceGatewayConfig `json:"localGatewayConfig,omitempty"` RemoteGatewayConfig SliceGatewayConfig `json:"remoteGatewayConfig,omitempty"` GatewayNumber int `json:"gatewayNumber,omitempty"` + // RouteEntireSliceSubnet, when true, tells the worker to route the whole + // slice subnet (not just the peer gateway's subnet) via this gateway. The + // controller sets it on a spoke's gateway to the hub in HubAndSpoke topology, + // so a spoke forwards all slice-internal traffic (including traffic destined + // for other spokes) to the hub, which relays it. + RouteEntireSliceSubnet bool `json:"routeEntireSliceSubnet,omitempty"` } type SliceGatewayConfig struct { diff --git a/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml b/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml index 8f47678e..04ba9885 100644 --- a/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml +++ b/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml @@ -123,6 +123,14 @@ spec: vpnIp: type: string type: object + routeEntireSliceSubnet: + description: |- + RouteEntireSliceSubnet, when true, tells the worker to route the whole + slice subnet (not just the peer gateway's subnet) via this gateway. The + controller sets it on a spoke's gateway to the hub in HubAndSpoke topology, + so a spoke forwards all slice-internal traffic (including traffic destined + for other spokes) to the hub, which relays it. + type: boolean sliceName: type: string type: object diff --git a/service/topology_resolver.go b/service/topology_resolver.go index 01cd674b..35d9b6a5 100644 --- a/service/topology_resolver.go +++ b/service/topology_resolver.go @@ -27,6 +27,10 @@ import ( type TopologyEdge struct { ServerCluster string ClientCluster string + // HubSpoke is true for a hub<->spoke edge in HubAndSpoke topology (the client + // side is a spoke connecting to its hub). It drives spoke-to-spoke routing: + // the spoke's gateway is told to route the entire slice subnet via the hub. + HubSpoke bool } // ResolveTopologyEdges computes the desired set of gateway connections for a @@ -57,7 +61,7 @@ func ResolveTopologyEdges(clusters []string, topology *controllerv1alpha1.Topolo } for _, hub := range topology.Hubs { for _, spoke := range spokes { - edges = append(edges, TopologyEdge{ServerCluster: hub, ClientCluster: spoke}) + edges = append(edges, TopologyEdge{ServerCluster: hub, ClientCluster: spoke, HubSpoke: true}) } } for i := 0; i < len(topology.Hubs); i++ { diff --git a/service/topology_resolver_test.go b/service/topology_resolver_test.go index 4358a911..8db901ad 100644 --- a/service/topology_resolver_test.go +++ b/service/topology_resolver_test.go @@ -55,8 +55,8 @@ func TestResolveTopologyEdges(t *testing.T) { clusters: []string{"worker-1", "worker-2", "worker-3"}, topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"worker-1"}}, want: []TopologyEdge{ - {ServerCluster: "worker-1", ClientCluster: "worker-2"}, - {ServerCluster: "worker-1", ClientCluster: "worker-3"}, + {ServerCluster: "worker-1", ClientCluster: "worker-2", HubSpoke: true}, + {ServerCluster: "worker-1", ClientCluster: "worker-3", HubSpoke: true}, }, }, { @@ -64,8 +64,8 @@ func TestResolveTopologyEdges(t *testing.T) { clusters: []string{"worker-1", "worker-2", "worker-3"}, topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"worker-2"}}, want: []TopologyEdge{ - {ServerCluster: "worker-2", ClientCluster: "worker-1"}, - {ServerCluster: "worker-2", ClientCluster: "worker-3"}, + {ServerCluster: "worker-2", ClientCluster: "worker-1", HubSpoke: true}, + {ServerCluster: "worker-2", ClientCluster: "worker-3", HubSpoke: true}, }, }, } diff --git a/service/worker_slice_gateway_service.go b/service/worker_slice_gateway_service.go index bcee202b..040d8260 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -473,7 +473,7 @@ func (s *WorkerSliceGatewayService) createMinimumGatewaysIfNotExists(ctx context } 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) + err := s.createMinimumGateWayPairIfNotExists(ctx, sourceCluster, destinationCluster, sliceName, namespace, sliceGwSvcType, gwSvcProtocol, ownerLabel, gatewayNumber, gatewayAddresses, edge.HubSpoke) if err != nil { return ctrl.Result{}, err } @@ -485,7 +485,7 @@ func (s *WorkerSliceGatewayService) createMinimumGatewaysIfNotExists(ctx context func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx context.Context, sourceCluster *controllerv1alpha1.Cluster, destinationCluster *controllerv1alpha1.Cluster, sliceName, namespace, gatewayConnType, gatewayProtocol string, label map[string]string, gatewayNumber int, - gatewayAddresses util.WorkerSliceGatewayNetworkAddresses) error { + gatewayAddresses util.WorkerSliceGatewayNetworkAddresses, routeEntireSliceSubnet bool) error { serverGatewayName := fmt.Sprintf(gatewayName, sliceName, sourceCluster.Name, destinationCluster.Name) clientGatewayName := fmt.Sprintf(gatewayName, sliceName, destinationCluster.Name, sourceCluster.Name) gateway := v1alpha1.WorkerSliceGateway{} @@ -548,6 +548,10 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont clientGateway, gatewayConnType, gatewayProtocol, label, gatewayNumber, gatewayAddresses.ClientSubnet, gatewayAddresses.ClientVpnAddress, serverGatewayName, gatewayAddresses.ServerSubnet, gatewayAddresses.ServerVpnAddress, clientGatewayName) + // For a hub-and-spoke edge the client side is the spoke; tell the worker to + // route the entire slice subnet via this gateway so spoke-to-spoke traffic is + // relayed through the hub. + clientGatewayObject.Spec.RouteEntireSliceSubnet = routeEntireSliceSubnet err = util.CreateResource(ctx, clientGatewayObject) if err != nil { //Register an event for worker slice gateway creation failure From 007b2c5b97ed7edb8ce15ebf25f2c1f220910431 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Mon, 3 Aug 2026 17:54:44 +0530 Subject: [PATCH 11/21] test: end-to-end hub-and-spoke topology builds partial mesh (#304) Signed-off-by: Shreesha001 --- .../sliceconfig_hubandspoke_test.go | 208 ++++++++++++++++++ 1 file changed, 208 insertions(+) create mode 100644 controllers/controller/sliceconfig_hubandspoke_test.go diff --git a/controllers/controller/sliceconfig_hubandspoke_test.go b/controllers/controller/sliceconfig_hubandspoke_test.go new file mode 100644 index 00000000..494a6dd3 --- /dev/null +++ b/controllers/controller/sliceconfig_hubandspoke_test.go @@ -0,0 +1,208 @@ +package controller + +import ( + "context" + + "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" + workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// End-to-end (envtest) test for the Hub-and-Spoke topology (#304). It drives the +// real SliceConfig reconciler and asserts that, for a HubAndSpoke slice with three +// clusters and worker-1 as the hub, the controller builds a partial mesh: only the +// hub<->spoke gateway links are created (no spoke<->spoke), the hub side is the +// Server and the spoke side the Client, and only the spoke gateways are marked to +// route the entire slice subnet via the hub. +var _ = Describe("SliceConfig HubAndSpoke topology (partial mesh)", Ordered, func() { + const ( + projectName = "hns" + nsName = "kubeslice-hns" + sliceName = "hns-slice" + ) + ctx := context.Background() + var project *v1alpha1.Project + + // register a Cluster and mark it Registered with a CNI subnet, the way the + // worker operator would, so the SliceConfig validation and gateway creation + // treat it as a live worker. + registerCluster := func(name, cniSubnet, nodeIP string) { + c := &v1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: nsName}, + Spec: v1alpha1.ClusterSpec{NodeIPs: []string{nodeIP}}, + } + Eventually(func() bool { + return k8sClient.Create(ctx, c) == nil + }, timeout, interval).Should(BeTrue()) + + key := types.NamespacedName{Namespace: nsName, Name: name} + Eventually(func() bool { + return k8sClient.Get(ctx, key, c) == nil + }, timeout, interval).Should(BeTrue()) + + c.Status.CniSubnet = []string{cniSubnet} + c.Status.NetworkPresent = true + c.Status.RegistrationStatus = v1alpha1.RegistrationStatusRegistered + Eventually(func() bool { + return k8sClient.Status().Update(ctx, c) == nil + }, timeout, interval).Should(BeTrue()) + } + + gatewayExists := func(name string) bool { + gw := workerv1alpha1.WorkerSliceGateway{} + return k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: nsName}, &gw) == nil + } + gatewayAbsent := func(name string) bool { + gw := workerv1alpha1.WorkerSliceGateway{} + return errors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: nsName}, &gw)) + } + getGateway := func(name string) workerv1alpha1.WorkerSliceGateway { + gw := workerv1alpha1.WorkerSliceGateway{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: nsName}, &gw)).Should(Succeed()) + return gw + } + + BeforeAll(func() { + project = &v1alpha1.Project{ + ObjectMeta: metav1.ObjectMeta{Name: projectName, Namespace: controlPlaneNamespace}, + } + Eventually(func() bool { + return k8sClient.Create(ctx, project) == nil + }, timeout, interval).Should(BeTrue()) + + ns := v1.Namespace{} + Eventually(func() bool { + return k8sClient.Get(ctx, types.NamespacedName{Name: nsName}, &ns) == nil + }, timeout, interval).Should(BeTrue()) + + registerCluster("worker-1", "192.168.0.0/24", "10.10.0.1") // hub + registerCluster("worker-2", "192.168.1.0/24", "10.10.0.2") // spoke + registerCluster("worker-3", "192.168.2.0/24", "10.10.0.3") // spoke + }) + + AfterAll(func() { + // Best-effort cleanup. This test uses its own project/namespace, so any + // leftovers don't affect other specs, and envtest tears everything down at + // suite end. Deboarding a slice fully (finalizers) is slow in envtest, so we + // don't block the suite on it. + slice := v1alpha1.SliceConfig{} + if k8sClient.Get(ctx, types.NamespacedName{Name: sliceName, Namespace: nsName}, &slice) == nil { + slice.Spec.Clusters = []string{} + _ = k8sClient.Update(ctx, &slice) + _ = k8sClient.Delete(ctx, &slice) + } + for _, name := range []string{"worker-1", "worker-2", "worker-3"} { + _ = k8sClient.Delete(ctx, &v1alpha1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: nsName}}) + } + _ = k8sClient.Delete(ctx, project) + }) + + It("builds only hub<->spoke gateways and skips spoke<->spoke", func() { + slice := &v1alpha1.SliceConfig{ + ObjectMeta: metav1.ObjectMeta{Name: sliceName, Namespace: nsName}, + Spec: v1alpha1.SliceConfigSpec{ + Clusters: []string{"worker-1", "worker-2", "worker-3"}, + MaxClusters: 4, + SliceSubnet: "10.7.0.0/16", + SliceGatewayProvider: &v1alpha1.WorkerSliceGatewayProvider{ + SliceGatewayType: "OpenVPN", + SliceCaType: "Local", + }, + SliceIpamType: "Local", + SliceType: "Application", + Topology: &v1alpha1.TopologySpec{ + Mode: v1alpha1.TopologyModeHubAndSpoke, + Hubs: []string{"worker-1"}, + }, + QosProfileDetails: &v1alpha1.QOSProfile{ + BandwidthCeilingKbps: 5120, + DscpClass: "AF11", + }, + }, + } + Expect(k8sClient.Create(ctx, slice)).Should(Succeed()) + + // the four hub<->spoke gateway links must be created + Eventually(func() bool { return gatewayExists(sliceName + "-worker-1-worker-2") }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return gatewayExists(sliceName + "-worker-2-worker-1") }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return gatewayExists(sliceName + "-worker-1-worker-3") }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return gatewayExists(sliceName + "-worker-3-worker-1") }, timeout, interval).Should(BeTrue()) + + // the spoke<->spoke links must never be created (partial mesh) + Consistently(func() bool { + return gatewayAbsent(sliceName+"-worker-2-worker-3") && gatewayAbsent(sliceName+"-worker-3-worker-2") + }, "3s", interval).Should(BeTrue()) + + // hub side is Server and does not route the entire subnet + hubGw := getGateway(sliceName + "-worker-1-worker-2") + Expect(hubGw.Spec.GatewayHostType).To(Equal("Server")) + Expect(hubGw.Spec.RouteEntireSliceSubnet).To(BeFalse()) + + // spoke side is Client and routes the entire slice subnet via the hub + spokeGw := getGateway(sliceName + "-worker-2-worker-1") + Expect(spokeGw.Spec.GatewayHostType).To(Equal("Client")) + Expect(spokeGw.Spec.RouteEntireSliceSubnet).To(BeTrue()) + }) + + It("full mesh is unaffected: builds all gateway pairs with no entire-subnet routing", func() { + // Same three clusters, but a FullMesh slice. This guards against a + // regression where the hub-and-spoke change leaks into the default + // topology: full mesh must still create every pair, and no gateway may be + // marked to route the entire slice subnet. + const fmName = "fm-slice" + slice := &v1alpha1.SliceConfig{ + ObjectMeta: metav1.ObjectMeta{Name: fmName, Namespace: nsName}, + Spec: v1alpha1.SliceConfigSpec{ + Clusters: []string{"worker-1", "worker-2", "worker-3"}, + MaxClusters: 4, + SliceSubnet: "10.8.0.0/16", + SliceGatewayProvider: &v1alpha1.WorkerSliceGatewayProvider{ + SliceGatewayType: "OpenVPN", + SliceCaType: "Local", + }, + SliceIpamType: "Local", + SliceType: "Application", + Topology: &v1alpha1.TopologySpec{Mode: v1alpha1.TopologyModeFullMesh}, + QosProfileDetails: &v1alpha1.QOSProfile{ + BandwidthCeilingKbps: 5120, + DscpClass: "AF11", + }, + }, + } + Expect(k8sClient.Create(ctx, slice)).Should(Succeed()) + + // all six gateway links (every pair, both directions) are created, + // including the spoke<->spoke pair that hub-and-spoke omits. + gwName := func(a, b string) string { return fmName + "-" + a + "-" + b } + for _, pair := range [][2]string{ + {"worker-1", "worker-2"}, {"worker-2", "worker-1"}, + {"worker-1", "worker-3"}, {"worker-3", "worker-1"}, + {"worker-2", "worker-3"}, {"worker-3", "worker-2"}, + } { + name := gwName(pair[0], pair[1]) + Eventually(func() bool { return gatewayExists(name) }, timeout, interval).Should(BeTrue(), name) + } + + // no gateway in a full-mesh slice routes the entire slice subnet + list := workerv1alpha1.WorkerSliceGatewayList{} + Expect(k8sClient.List(ctx, &list, client.InNamespace(nsName))).Should(Succeed()) + for i := range list.Items { + gw := &list.Items[i] + if gw.Spec.SliceName == fmName { + Expect(gw.Spec.RouteEntireSliceSubnet).To(BeFalse(), gw.Name) + } + } + + // cleanup this slice (best-effort) + slice.Spec.Clusters = []string{} + _ = k8sClient.Update(ctx, slice) + _ = k8sClient.Delete(ctx, slice) + }) +}) From c7b736b3f44a073d6416817837c047a1d877b44f Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Sat, 8 Aug 2026 12:45:46 +0530 Subject: [PATCH 12/21] fix: reconcile RouteEntireSliceSubnet on surviving gateways when topology changes Signed-off-by: Shreesha001 --- .../sliceconfig_hubandspoke_test.go | 65 +++++++++++++++++++ service/worker_slice_gateway_service.go | 11 ++++ 2 files changed, 76 insertions(+) diff --git a/controllers/controller/sliceconfig_hubandspoke_test.go b/controllers/controller/sliceconfig_hubandspoke_test.go index 494a6dd3..bf9fef95 100644 --- a/controllers/controller/sliceconfig_hubandspoke_test.go +++ b/controllers/controller/sliceconfig_hubandspoke_test.go @@ -205,4 +205,69 @@ var _ = Describe("SliceConfig HubAndSpoke topology (partial mesh)", Ordered, fun _ = k8sClient.Update(ctx, slice) _ = k8sClient.Delete(ctx, slice) }) + + It("reconciles RouteEntireSliceSubnet when the topology changes on an existing slice", func() { + // A slice that starts as FullMesh and is later switched to HubAndSpoke. + // The spoke<->hub edge exists in both topologies, so its gateway is NOT + // recreated on the change. This guards the bug where RouteEntireSliceSubnet + // was only written at creation and left stale on the surviving gateway, + // which silently breaks spoke-to-spoke on an upgraded slice. + const tName = "shift-slice" + key := types.NamespacedName{Name: tName, Namespace: nsName} + gwName := tName + "-worker-2-worker-1" // spoke-2 -> worker-1 (hub-to-be) + + slice := &v1alpha1.SliceConfig{ + ObjectMeta: metav1.ObjectMeta{Name: tName, Namespace: nsName}, + Spec: v1alpha1.SliceConfigSpec{ + Clusters: []string{"worker-1", "worker-2", "worker-3"}, + MaxClusters: 4, + SliceSubnet: "10.9.0.0/16", + SliceGatewayProvider: &v1alpha1.WorkerSliceGatewayProvider{ + SliceGatewayType: "OpenVPN", + SliceCaType: "Local", + }, + SliceIpamType: "Local", + SliceType: "Application", + Topology: &v1alpha1.TopologySpec{Mode: v1alpha1.TopologyModeFullMesh}, + QosProfileDetails: &v1alpha1.QOSProfile{ + BandwidthCeilingKbps: 5120, + DscpClass: "AF11", + }, + }, + } + Expect(k8sClient.Create(ctx, slice)).Should(Succeed()) + + // full mesh: the gateway exists and does not route the entire subnet + Eventually(func() bool { return gatewayExists(gwName) }, timeout, interval).Should(BeTrue()) + Expect(getGateway(gwName).Spec.RouteEntireSliceSubnet).To(BeFalse()) + uidBefore := getGateway(gwName).UID + + // switch to HubAndSpoke with worker-1 as the hub + latest := &v1alpha1.SliceConfig{} + Expect(k8sClient.Get(ctx, key, latest)).Should(Succeed()) + latest.Spec.Topology = &v1alpha1.TopologySpec{Mode: v1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"worker-1"}} + Expect(k8sClient.Update(ctx, latest)).Should(Succeed()) + + // the surviving spoke->hub gateway must now route the entire slice subnet + Eventually(func() bool { + return getGateway(gwName).Spec.RouteEntireSliceSubnet + }, timeout, interval).Should(BeTrue()) + // and it must be the same object, reconciled in place, not recreated + Expect(getGateway(gwName).UID).To(Equal(uidBefore)) + + // switch back to FullMesh: the flag must be cleared again + Expect(k8sClient.Get(ctx, key, latest)).Should(Succeed()) + latest.Spec.Topology = &v1alpha1.TopologySpec{Mode: v1alpha1.TopologyModeFullMesh} + Expect(k8sClient.Update(ctx, latest)).Should(Succeed()) + Eventually(func() bool { + return getGateway(gwName).Spec.RouteEntireSliceSubnet + }, timeout, interval).Should(BeFalse()) + + // cleanup (best-effort) + if k8sClient.Get(ctx, key, latest) == nil { + latest.Spec.Clusters = []string{} + _ = k8sClient.Update(ctx, latest) + _ = k8sClient.Delete(ctx, latest) + } + }) }) diff --git a/service/worker_slice_gateway_service.go b/service/worker_slice_gateway_service.go index 040d8260..6ccd2489 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -502,6 +502,17 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont return err } if found { + // The gateway pair already exists. On a topology change the surviving + // spoke<->hub edge is not recreated, so RouteEntireSliceSubnet would go + // stale (e.g. a FullMesh->HubAndSpoke switch would leave it false and + // silently break spoke-to-spoke). Reconcile it on the existing client + // gateway instead of returning early. + if gateway.Spec.RouteEntireSliceSubnet != routeEntireSliceSubnet { + gateway.Spec.RouteEntireSliceSubnet = routeEntireSliceSubnet + if err = util.UpdateResource(ctx, &gateway); err != nil { + return err + } + } return nil } } From ed4d7dd92daf9cb20db38bdef9127fd09cd5d27f Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Sat, 8 Aug 2026 12:45:46 +0530 Subject: [PATCH 13/21] feat: add gateway connection status fields to WorkerSliceGateway CRD Signed-off-by: Shreesha001 --- .../v1alpha1/workerslicegateway_types.go | 22 +++++++++++++++++++ apis/worker/v1alpha1/zz_generated.deepcopy.go | 6 ++++- ...rker.kubeslice.io_workerslicegateways.yaml | 17 ++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/apis/worker/v1alpha1/workerslicegateway_types.go b/apis/worker/v1alpha1/workerslicegateway_types.go index 3b88762b..ba16ec7c 100644 --- a/apis/worker/v1alpha1/workerslicegateway_types.go +++ b/apis/worker/v1alpha1/workerslicegateway_types.go @@ -67,9 +67,31 @@ type GatewayCredentials struct { } // WorkerSliceGatewayStatus defines the observed state of WorkerSliceGateway +// Gateway connection states reported by the worker on WorkerSliceGatewayStatus. +const ( + // GatewayConnectionStateConnected means the gateway tunnel is up (at least + // one HA gateway pod reports its tunnel established). + GatewayConnectionStateConnected = "Connected" + // GatewayConnectionStateNotConnected means the tunnel is down (all gateway + // pods report their tunnel not established). + GatewayConnectionStateNotConnected = "NotConnected" + // GatewayConnectionStatePending means no connectivity has been reported yet. + // An empty ConnectionState is treated as Pending by the controller-side aggregation. + GatewayConnectionStatePending = "Pending" +) + type WorkerSliceGatewayStatus struct { GatewayNumber int `json:"gatewayNumber,omitempty"` ClusterInsertionIndex int `json:"clusterInsertionIndex,omitempty"` + // ConnectionState is the connectivity state of this gateway link as reported + // by the worker: Connected, NotConnected or Pending. Empty means Pending. + ConnectionState string `json:"connectionState,omitempty"` + // LastTransitionTime is the time ConnectionState last changed. + LastTransitionTime *metav1.Time `json:"lastTransitionTime,omitempty"` + // Reason is a short, machine-readable reason for the current ConnectionState. + Reason string `json:"reason,omitempty"` + // Message is a human-readable description of the current ConnectionState. + Message string `json:"message,omitempty"` } //+kubebuilder:object:root=true diff --git a/apis/worker/v1alpha1/zz_generated.deepcopy.go b/apis/worker/v1alpha1/zz_generated.deepcopy.go index 578ce988..8d9fc229 100644 --- a/apis/worker/v1alpha1/zz_generated.deepcopy.go +++ b/apis/worker/v1alpha1/zz_generated.deepcopy.go @@ -497,7 +497,7 @@ func (in *WorkerSliceGateway) DeepCopyInto(out *WorkerSliceGateway) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkerSliceGateway. @@ -586,6 +586,10 @@ func (in *WorkerSliceGatewaySpec) DeepCopy() *WorkerSliceGatewaySpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *WorkerSliceGatewayStatus) DeepCopyInto(out *WorkerSliceGatewayStatus) { *out = *in + if in.LastTransitionTime != nil { + in, out := &in.LastTransitionTime, &out.LastTransitionTime + *out = (*in).DeepCopy() + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkerSliceGatewayStatus. diff --git a/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml b/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml index 04ba9885..4ad9a4e8 100644 --- a/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml +++ b/config/crd/bases/worker.kubeslice.io_workerslicegateways.yaml @@ -139,8 +139,25 @@ spec: properties: clusterInsertionIndex: type: integer + connectionState: + description: 'ConnectionState is the connectivity state of this gateway + link as reported by the worker: Connected, NotConnected or Pending. + Empty means Pending.' + type: string gatewayNumber: type: integer + lastTransitionTime: + description: LastTransitionTime is the time ConnectionState last changed. + format: date-time + type: string + message: + description: Message is a human-readable description of the current + ConnectionState. + type: string + reason: + description: Reason is a short, machine-readable reason for the current + ConnectionState. + type: string type: object type: object served: true From 502904c0dad5ea430afa2ec36adcfcef931169f4 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Mon, 24 Aug 2026 18:32:50 +0530 Subject: [PATCH 14/21] fix: clear RouteEntireSliceSubnet on server gateway after hub change Signed-off-by: Shreesha001 --- service/worker_slice_gateway_service.go | 41 ++++++++++++++------ service/worker_slice_gateway_service_test.go | 13 +++++++ 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/service/worker_slice_gateway_service.go b/service/worker_slice_gateway_service.go index 6ccd2489..f950c6d8 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -481,6 +481,18 @@ func (s *WorkerSliceGatewayService) createMinimumGatewaysIfNotExists(ctx context return ctrl.Result{}, nil } +// reconcileRouteEntireSliceSubnet sets a gateway's RouteEntireSliceSubnet flag to +// the desired value only when it differs, keeping the write idempotent across +// reconciles. It is used to keep the flag correct on an existing gateway pair +// after a topology change (see createMinimumGateWayPairIfNotExists). +func (s *WorkerSliceGatewayService) reconcileRouteEntireSliceSubnet(ctx context.Context, gateway *v1alpha1.WorkerSliceGateway, desired bool) error { + if gateway.Spec.RouteEntireSliceSubnet == desired { + return nil + } + gateway.Spec.RouteEntireSliceSubnet = desired + return util.UpdateResource(ctx, gateway) +} + // 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, @@ -488,30 +500,35 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont gatewayAddresses util.WorkerSliceGatewayNetworkAddresses, routeEntireSliceSubnet bool) error { serverGatewayName := fmt.Sprintf(gatewayName, sliceName, sourceCluster.Name, destinationCluster.Name) clientGatewayName := fmt.Sprintf(gatewayName, sliceName, destinationCluster.Name, sourceCluster.Name) - gateway := v1alpha1.WorkerSliceGateway{} - found, err := util.GetResourceIfExist(ctx, client.ObjectKey{Name: serverGatewayName, Namespace: namespace}, &gateway) + serverGw := v1alpha1.WorkerSliceGateway{} + found, err := util.GetResourceIfExist(ctx, client.ObjectKey{Name: serverGatewayName, Namespace: namespace}, &serverGw) if err != nil { return err } if found { + clientGw := v1alpha1.WorkerSliceGateway{} found, err = util.GetResourceIfExist(ctx, client.ObjectKey{ Name: clientGatewayName, Namespace: namespace, - }, &gateway) + }, &clientGw) if err != nil { return err } if found { // The gateway pair already exists. On a topology change the surviving - // spoke<->hub edge is not recreated, so RouteEntireSliceSubnet would go - // stale (e.g. a FullMesh->HubAndSpoke switch would leave it false and - // silently break spoke-to-spoke). Reconcile it on the existing client - // gateway instead of returning early. - if gateway.Spec.RouteEntireSliceSubnet != routeEntireSliceSubnet { - gateway.Spec.RouteEntireSliceSubnet = routeEntireSliceSubnet - if err = util.UpdateResource(ctx, &gateway); err != nil { - return err - } + // edge is not recreated, so RouteEntireSliceSubnet can go stale and + // must be reconciled on both sides: + // - the client (spoke) side must be SET to routeEntireSliceSubnet, e.g. + // a FullMesh->HubAndSpoke switch would otherwise leave it false and + // silently break spoke-to-spoke; and + // - the server (hub) side must be CLEARED to false: a hub change can + // turn a former client gateway (flag true) into a server, and a + // server/hub that routes the whole slice would misroute all traffic. + if err = s.reconcileRouteEntireSliceSubnet(ctx, &clientGw, routeEntireSliceSubnet); err != nil { + return err + } + if err = s.reconcileRouteEntireSliceSubnet(ctx, &serverGw, false); err != nil { + return err } return nil } diff --git a/service/worker_slice_gateway_service_test.go b/service/worker_slice_gateway_service_test.go index d3ab85b2..5b2ca81e 100644 --- a/service/worker_slice_gateway_service_test.go +++ b/service/worker_slice_gateway_service_test.go @@ -362,6 +362,13 @@ func testCreateMinimumWorkerSliceGatewaysHubAndSpokeSkipsSpokeToSpoke(t *testing // all found -> nothing created. A spoke<->spoke pair would exceed 4 checks. gateway := &workerv1alpha1.WorkerSliceGateway{} clientMock.On("Get", ctx, mock.AnythingOfType("types.NamespacedName"), gateway).Return(nil).Times(4) + // each existing hub<->spoke client gateway has its RouteEntireSliceSubnet + // reconciled to true (the mock returns the default false), one Update per + // hub<->spoke edge; the server side is already false so it is a no-op. + clientMock.On("Update", ctx, mock.AnythingOfType("*v1alpha1.WorkerSliceGateway")).Return(nil).Run(func(args mock.Arguments) { + gw := args.Get(1).(*workerv1alpha1.WorkerSliceGateway) + require.True(t, gw.Spec.RouteEntireSliceSubnet) + }).Twice() result, err := workerSliceGatewayService.CreateMinimumWorkerSliceGateways(ctx, "red", clusterNames, requestObj.Namespace, label, clusterMap, "10.10.10.10/16", "/16", nil, topology) require.Equal(t, ctrl.Result{}, result) @@ -425,6 +432,12 @@ func testCreateMinimumWorkerSliceGatewaysHubAndSpokeCleansUpSpokeToSpoke(t *test clientMock.On("Get", ctx, mock.AnythingOfType("types.NamespacedName"), cluster).Return(nil).Times(3) gateway := &workerv1alpha1.WorkerSliceGateway{} clientMock.On("Get", ctx, mock.AnythingOfType("types.NamespacedName"), gateway).Return(nil).Times(4) + // surviving hub<->spoke client gateways get RouteEntireSliceSubnet reconciled + // to true (mock returns default false); server side already false = no-op. + clientMock.On("Update", ctx, mock.AnythingOfType("*v1alpha1.WorkerSliceGateway")).Return(nil).Run(func(args mock.Arguments) { + gw := args.Get(1).(*workerv1alpha1.WorkerSliceGateway) + require.True(t, gw.Spec.RouteEntireSliceSubnet) + }).Twice() result, err := workerSliceGatewayService.CreateMinimumWorkerSliceGateways(ctx, "red", clusterNames, requestObj.Namespace, label, clusterMap, "10.10.10.10/16", "/16", nil, topology) require.Equal(t, ctrl.Result{}, result) From 9b45b12eec22bcb5e2258882006c35d4260c02a3 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Wed, 26 Aug 2026 11:58:52 +0530 Subject: [PATCH 15/21] test: add hub-and-spoke control-plane e2e suite and testing docs Signed-off-by: Shreesha001 --- Makefile | 4 + docs/hub-and-spoke-testing.md | 450 +++++++++++++++++++++++++++++++ test/e2e/harness_test.go | 234 ++++++++++++++++ test/e2e/hubandspoke_e2e_test.go | 75 ++++++ test/e2e/scenarios_test.go | 183 +++++++++++++ 5 files changed, 946 insertions(+) create mode 100644 docs/hub-and-spoke-testing.md create mode 100644 test/e2e/harness_test.go create mode 100644 test/e2e/hubandspoke_e2e_test.go create mode 100644 test/e2e/scenarios_test.go diff --git a/Makefile b/Makefile index 8ac118cc..c19fd5a3 100644 --- a/Makefile +++ b/Makefile @@ -83,6 +83,10 @@ test: manifests generate fmt vet envtest ## Run tests. test-local: envtest ## Run tests. KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./controllers/controller/... -coverprofile cover.out +.PHONY: test-e2e-hns +test-e2e-hns: ## Run the Hub-and-Spoke (partial mesh) control-plane e2e suite against a real, disposable Kind cluster. + go test -tags e2e -v -timeout 20m ./test/e2e/... + .PHONY: int-test int-test: envtest KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./controllers/controller/... -coverprofile cover.out diff --git a/docs/hub-and-spoke-testing.md b/docs/hub-and-spoke-testing.md new file mode 100644 index 00000000..9319b8f0 --- /dev/null +++ b/docs/hub-and-spoke-testing.md @@ -0,0 +1,450 @@ +# Hub-and-Spoke (Partial Mesh) test suite + +Every test that covers the **Hub-and-Spoke partial-mesh topology** and its +**spoke-to-spoke routing** (issues #300–#304, #471), organized by what it +verifies and how to run it. It also documents the manual dataplane +end-to-end scenarios that were run on real Kind clusters, with the exact +commands and observed results. + +**Transport scope:** this document covers the feature on **OpenVPN**, which is +fully tested end-to-end. WireGuard is **deferred** — see +[Section 7 Testing notes & coverage gaps](#7-testing-notes--coverage-gaps) for why the WireGuard dataplane +cannot come up yet (a pre-existing controller key-generation gap, outside this +feature). + +The feature spans three repositories: + +| Repo | Role | +|---|---| +| `kubeslice-controller` | Topology API + webhook validation, edge computation, marks each spoke's gateway with `RouteEntireSliceSubnet`, and aggregates gateway health into the SliceConfig `TopologyConverged` status | +| `worker-operator` | Programs the entire-slice route on **both** the spoke gateway pod and the slice router, and reports each gateway's tunnel connectivity back to the hub | +| `gateway-sidecar` | Installs the tunnel route as two more-specific halves so NSM cannot overwrite it | +| `apis` | Shared CRD types — the topology fields and the gateway connection-status fields | + +--- + +## Layout + +Coverage is layered; each layer catches a different class of bug and runs at a +different cost: + +| Layer | Location | What it needs | +|---|---|---| +| Unit | `service/*_test.go` (controller), `controllers/slicegateway/*_test.go` (worker), `pkg/sidecar/sidecarpb/*_test.go` (sidecar) | nothing (pure Go / fake mocks) | +| Reconciler (envtest) | `controllers/controller/sliceconfig_hubandspoke_test.go` (controller) | envtest `etcd`/`kube-apiserver` binaries | +| Control-plane E2E (automated) | `test/e2e/*_test.go`, build tag `e2e` — run with `make test-e2e-hns` | `docker`, `kind`, `kubectl`, `make`; one disposable Kind cluster | +| Dataplane E2E (manual runbook) | [Section 5](#5-dataplane-end-to-end-runbook-openvpn) | `docker`, `kind`, `kubectl`, `helm`; real disposable Kind clusters | + +The automated control-plane E2E suite (`test/e2e/`) builds this branch's +controller image, spins up **one** disposable Kind cluster prefixed `e2e-hns-` +(created and deleted within the run via `t.Cleanup`, never touching your own +clusters), runs the real controller (reconcilers + webhooks + CRDs) against +three fake pre-registered worker Cluster CRs, and asserts the control-plane +behaviour. It does **not** install real workers/NSM/gateways — that (the +dataplane) is the manual runbook in [Section 5](#5-dataplane-end-to-end-runbook-openvpn). + +```bash +make test-e2e-hns # go test -tags e2e -v -timeout 20m ./test/e2e/... +``` + +Scenarios (all pass; ~4 min total): + +| Subtest | Asserts | +|---|---| +| `PartialMesh_HubAndSpokeSkipsSpokeToSpoke` | 4 gateways; hub=Server/``, spoke=Client/`true`; no spoke↔spoke | +| `FullMesh_Unaffected` | full mesh → 6 gateways, no gateway carries the flag | +| `TopologyChange_ReconcilesFlag` | FullMesh→HubAndSpoke re-sets the spoke flag to `true` | +| `HubChange_NoStaleServerFlag` | hub-change round-trip leaves no Server gateway with `route=true` | +| `Webhook_RejectsInvalidTopologies` | all 7 invalid topologies rejected at admission | +| `StatusFields_Persist` | #471 connection-status fields survive a status write | + +Run the pure unit layer: + +```bash +# controller topology + webhook + gateway-creation unit tests +cd kubeslice-controller && go test ./service/ -run 'TestResolveTopologyEdges|TestTopologyEdgeSetContains|test_validateTopology|HubAndSpoke' + +# controller TopologyConverged status aggregation (#303 branch) +cd kubeslice-controller && go test ./service/ -run TestBuildTopologyConvergedCondition + +# worker-operator route selection +cd worker-operator && go test ./controllers/slicegateway/ -run TestRemoteNsmSubnetForRoute + +# worker-operator flag propagation (hub controller) +cd worker-operator && go test ./pkg/hub/controllers/ -run 'TestNewMeshGatewayConfig|TestStaticGatewayConfigChanged' + +# worker-operator gateway connection status (feature/471-report-gateway-status) +cd worker-operator && go test ./pkg/hub/controllers/ -run 'TestDeriveGatewayConnectionState|TestReconcileGatewayConnectionStatus|TestReasonMessageForState' + +# gateway-sidecar route split + MSS clamp +cd gateway-sidecar && go test ./pkg/sidecar/sidecarpb/ -run 'TestMoreSpecificHalves|TestTunnelMSSClampCommands' +``` + +> **Known gate — the controller `service` package.** On `master` today the +> `service` test binary does **not compile standalone**, for two reasons +> unrelated to this feature: `util.Client` is undefined and +> `ObjectMeta.ClusterName` was removed upstream. Both are fixed by **PR #404** +> (`fix/test-type-mismatch`). With #404 applied, the `service` commands above +> compile and pass (verified locally). The worker-operator and gateway-sidecar +> commands pass today as-is. + +Run the reconciler (envtest) layer — needs envtest assets: + +```bash +cd kubeslice-controller +export KUBEBUILDER_ASSETS=$(setup-envtest use 1.26.1 -p path) # or your local assets path +go test ./controllers/controller/ -args -ginkgo.focus="HubAndSpoke topology" +``` + +The full `controllers/controller` suite also contains unrelated, occasionally +flaky specs (`vpnkey_rotation_controller_test.go`); the +`-ginkgo.focus="HubAndSpoke topology"` filter runs only this feature's specs. + +--- + +## 1. Controller unit tests: topology edge computation + +File: `service/topology_resolver_test.go`. Pure functions, no mocks. + +### `TestResolveTopologyEdges` — 4 cases + +`ResolveTopologyEdges(clusters, topology)` returns the desired gateway edges. + +- **`nil topology is full mesh in cluster order`** — no topology → every cluster + pair, in list order, with the entire-subnet flag OFF. The backward-compat + guarantee: existing slices are unchanged. +- **`explicit FullMesh is full mesh`** — `mode: FullMesh` behaves identically to + no topology. +- **`hub and spoke: hub is server, no spoke-to-spoke`** — hub=worker-1 → + produces `worker-1↔worker-2` and `worker-1↔worker-3` (hub as server, flag ON + on the spoke side) and **never** `worker-2↔worker-3`. +- **`hub is server even when not first in the cluster list`** — hub role is + driven by `topology.hubs`, not by cluster ordering. + +### `TestTopologyEdgeSetContains` + +The desired-edge set is **direction-insensitive**: for hub=worker-1, both +`Contains("worker-1","worker-2")` and `Contains("worker-2","worker-1")` are +true (server-side and client-side gateway of a pair map to the same edge), and +the spoke↔spoke pair `worker-2↔worker-3` is **not** a member in either +direction. This is what lets cleanup delete a stale spoke↔spoke pair without +accidentally deleting one side of a desired pair. + +--- + +## 2. Controller unit tests: topology webhook validation + +File: `service/slice_config_webhook_validation_test.go` → `test_validateTopology`. +Wired into **both** `ValidateSliceConfigCreate` and `ValidateSliceConfigUpdate`, +so a topology change on a live slice is validated too. + +10 table-driven cases — 3 accepted, 7 rejected (with the exact error text +asserted): + +| # | Case | Result | Error contains | +|---|---|---|---| +| 1 | absent topology (full mesh default) | accept | — | +| 2 | explicit FullMesh without hubs | accept | — | +| 3 | valid HubAndSpoke with one hub | accept | — | +| 4 | HubAndSpoke without hubs | reject | `requires at least one hub` | +| 5 | hub not a member of clusters | reject | `is not a member of spec.clusters` | +| 6 | single-cluster HubAndSpoke | reject | `requires at least 2 clusters` | +| 7 | FullMesh with hubs | reject | `hubs must be empty when mode is FullMesh` | +| 8 | more than one hub (single-hub MVP) | reject | `only one hub is supported in this release` | +| 9 | unknown mode | reject | `unknown topology mode` | +| 10 | hubs without mode | reject | `mode must be set to HubAndSpoke when hubs is specified` | + +Note: the single-hub restriction (case 8) is ordered **before** the duplicate +check, so `[worker-1, worker-1]` reports "only one hub is supported" (the more +specific error) rather than "duplicate". + +--- + +## 3. Controller unit tests: gateway creation (partial mesh) + +File: `service/worker_slice_gateway_service_test.go`. Uses the testify client +mock; asserts the exact set of gateway create / delete / update calls. + +- **`TestCreateMinimumWorkerSliceGateways_HubAndSpokeSkipsSpokeToSpoke`** — for a + HubAndSpoke slice with 3 clusters, exactly the two hub↔spoke pairs are + processed (3 cluster fetches, 4 gateway existence checks); the spoke↔spoke + pair is never created; each surviving spoke client gateway has + `RouteEntireSliceSubnet` reconciled to `true`. +- **`TestCreateMinimumWorkerSliceGateways_HubAndSpokeCleansUpSpokeToSpoke`** — a + pre-existing spoke↔spoke gateway pair (e.g. left from a FullMesh→HubAndSpoke + switch), given the *correct* gateway number and with both clusters still + members, is deleted **purely** because its edge is no longer in the desired + topology. + +**Regression (hub-change flag):** the reconcile now runs on **both** sides of an +existing pair — the client is set to the desired value, and the **server is +forced to `false`**. Without this, a hub change that turns a former client +gateway (flag `true`) into a server would leave a stale `true` on the hub side, +making the hub route the entire slice and misdirect all traffic. Fixed in +`reconcileRouteEntireSliceSubnet`; verified live in [Section 5 E9](#5-dataplane-end-to-end-runbook-openvpn). + +--- + +## 3a. Controller unit tests: TopologyConverged status aggregation (#303) + +File: `service/topology_status_test.go` → `TestBuildTopologyConvergedCondition`. +`buildTopologyConvergedCondition` folds every gateway's connection state into a +single `TopologyConverged` condition on the SliceConfig. 5 cases: + +- **`no gateways is trivially converged`** — an empty gateway set converges. +- **`all connected is converged`** — every gateway `Connected` → converged. +- **`one not connected is not converged and is named`** — a single down gateway + flips the condition and the message names it. +- **`empty connection state is reported as Pending`** — an unset state is treated + as `Pending`, not silently "up". +- **`first not-ready is chosen deterministically by name`** — when several are + not ready, the one reported is chosen by name so the message is stable. + +--- + +## 4. Controller reconciler tests (envtest) + +File: `controllers/controller/sliceconfig_hubandspoke_test.go` (Ginkgo, real API +server via envtest). 3 specs: + +- **`builds only hub<->spoke gateways and skips spoke<->spoke`** — applying a + HubAndSpoke SliceConfig creates the 4 WorkerSliceGateway objects (2 pairs) and + no spoke↔spoke object. +- **`full mesh is unaffected: builds all gateway pairs with no entire-subnet + routing`** — a FullMesh (or no-topology) slice builds all pairs and never sets + `RouteEntireSliceSubnet`. +- **`reconciles RouteEntireSliceSubnet when the topology changes on an existing + slice`** — switching an existing slice's topology updates the flag on the + surviving gateways (the control-plane half of the E8/E9 dataplane scenarios). + +--- + +## 4a. Worker-operator unit tests: route selection & flag propagation + +**Route selection** — file: `controllers/slicegateway/slicegateway_route_test.go` +→ `TestRemoteNsmSubnetForRoute`. 3 cases: + +- **full-mesh / normal gateway** → routes the **peer** gateway's subnet. +- **spoke→hub gateway** → routes the **entire slice** subnet (so the spoke sends + all slice traffic, including other spokes', to the hub). +- **spoke→hub gateway, slice subnet unknown** → returns **not ready**, telling + the caller to requeue instead of programming a wrong route. + +**Flag propagation** — file: `pkg/hub/controllers/slicegateway_config_test.go`. +The hub controller copies the controller-set `RouteEntireSliceSubnet` from the +`WorkerSliceGateway` spec onto the local `SliceGateway.Status.Config` that the +dataplane reads: + +- **`TestNewMeshGatewayConfig_PropagatesRouteEntireSliceSubnet`** — the flag is + copied in both states (`true`/`false`), along with the other config fields. +- **`TestStaticGatewayConfigChanged_DetectsRouteFlag`** — a flag flip is detected + as a change (so the worker re-syncs when the controller toggles it), and an + already-matching config reports no change (no status churn). + +## 4b. Gateway-sidecar unit tests: tunnel route split & MSS clamp + +File: `pkg/sidecar/sidecarpb/route_split_test.go`. + +**`TestMoreSpecificHalves`** — the route split, 3 cases: + +- `10.11.0.0/16` → `10.11.0.0/17` + `10.11.128.0/17` +- `10.11.0.0/20` → `10.11.0.0/21` + `10.11.8.0/21` +- `10.11.32.3/32` → unchanged (a host route is not split) + +**Why split at all:** NSM continuously re-asserts a route for the whole slice +subnet via `nsm0` using the *same* prefix the tunnel wants. A `RouteReplace` on +that exact prefix only wins until NSM re-asserts, so the route flaps and +spoke-to-spoke traffic intermittently black-holes. Installing two strictly +more-specific halves means longest-prefix match always selects the tunnel and +NSM can never overwrite it. + +**`TestTunnelMSSClampCommands`** — the TCP MSS clamp rule. Verifies the +`iptables` command targets the mangle `FORWARD` chain on the given interface +with `--clamp-mss-to-pmtu`, that the check (`-C`) and add (`-A`) commands differ +only in that verb, and that the interface is parameterized (correct for any +tunnel, not hard-wired). The clamp keeps full-size TCP segments from +black-holing across the smaller-MTU tunnel — needed for spoke-to-spoke, which +crosses two tunnels. + +## 4c. Worker-operator unit tests: gateway connection status (#471) + +File: `pkg/hub/controllers/slicegateway_status_test.go` (worker-operator). The +worker derives each gateway's tunnel state from its pod statuses and reports it +onto the hub's `WorkerSliceGateway` status — the write side of the fields +checked live in [Section 5 E12](#5-dataplane-end-to-end-runbook-openvpn). + +- **`TestDeriveGatewayConnectionState`** — 6 cases: no pod status → `Pending`; + all pods up → `Connected`; at least one up → `Connected` (HA pair); all down → + `NotConnected`; unknown/empty pod states are not counted as up; nil pod entries + are ignored. +- **`TestReconcileGatewayConnectionStatus`** — 2 subtests: writes `Connected` + when a tunnel is up; **no write when the state is unchanged** (idempotent — + avoids status churn). +- **`TestReasonMessageForState`** — the state → (reason, message) mapping, e.g. + `Connected` → (`TunnelEstablished`, `gateway tunnel is up`). + +--- + +## 5. Dataplane end-to-end runbook (OpenVPN) + +Real setup: 1 controller + 3 worker Kind clusters, NSM, real OpenVPN gateways, +running the feature images. Slice `10.11.0.0/16`, `HubAndSpoke`, hub = worker-1, +spokes = worker-2 (`10.11.16.x`) and worker-3 (`10.11.32.x`). + +> Contexts below: `kind-kubeslice-controller`, `kind-kubeslice-worker1|2|3`. +> `CPOD` = the `iperf-sleep` client pod on worker-2; `SIP` = the `iperf-server` +> pod's slice IP on worker-3. + +### E1 — Gateway roles + flag +```bash +kubectl get workerslicegateways -n kubeslice-avesha --context kind-kubeslice-controller \ + -o custom-columns=NAME:.metadata.name,HOST:.spec.gatewayHostType,ROUTE:.spec.routeEntireSliceSubnet +``` +**Expect:** 4 gateways — hub (`worker-1-worker-*`) = `Server` / ``, spoke +(`worker-2|3-worker-1`) = `Client` / `true`. + +### E2 — No spoke↔spoke gateway +```bash +kubectl get workerslicegateway demo-hub-and-spoke-worker-2-worker-3 -n kubeslice-avesha --context kind-kubeslice-controller +``` +**Expect:** `NotFound`. + +### E3 — Route split is live (the core fix) +```bash +kubectl exec -n kubeslice-system -c kubeslice-sidecar \ + --context kind-kubeslice-worker2 -- ip route +``` +**Expect:** `10.11.0.0/17` and `10.11.128.0/17 via 10.11.255.1 dev tun0`, +out-specifying NSM's `10.11.0.0/16 via ... nsm0`. + +### E4 — Spoke→spoke ping (relayed via hub) +```bash +kubectl exec $CPOD -n iperf -c sidecar --context kind-kubeslice-worker2 -- ping -c 5 $SIP +``` +**Expect:** 0% packet loss; `ttl` reduced (a hop through the hub). Verified: `5 received, 0% packet loss`. + +### E5 — Spoke→spoke iperf (throughput) +```bash +kubectl exec $CPOD -n iperf -c iperf --context kind-kubeslice-worker2 -- iperf -c $SIP -p 5201 -t 8 -i 2 +``` +**Expect:** a steady TCP transfer. Verified: ~4.4 Mbit/s (a local-Kind number, +not a benchmark). + +### E6 — HA gateway failover +```bash +# with a continuous ping running, delete the ACTIVE spoke gateway pod: +kubectl delete po -n kubeslice-system --context kind-kubeslice-worker2 +``` +**Expect:** killing the **standby** = 0% loss; killing the **active** = ~15–20 s +disruption then self-recovers; the pod is replaced automatically. Verified: +~18 s window, recovered to 0% loss, pair back to `Running`. + +### E7 — Tunnel down via firewall rule +```bash +# block on BOTH spoke-1 gateway pods: +kubectl exec -n kubeslice-system -c kubeslice-sidecar --context kind-kubeslice-worker2 -- iptables -I FORWARD -o tun0 -j DROP +# ... ping now fails ... +kubectl exec -n kubeslice-system -c kubeslice-sidecar --context kind-kubeslice-worker2 -- iptables -D FORWARD -o tun0 -j DROP +``` +**Expect:** 100% loss while blocked → 0% after removing the rule. Verified. + +### E8 — Topology change: FullMesh ↔ HubAndSpoke +```bash +# to full mesh: +kubectl patch sliceconfig demo-hub-and-spoke -n kubeslice-avesha --context kind-kubeslice-controller --type=json -p='[{"op":"remove","path":"/spec/topology"}]' +# back to hub-and-spoke: +kubectl patch sliceconfig demo-hub-and-spoke -n kubeslice-avesha --context kind-kubeslice-controller --type=merge -p='{"spec":{"topology":{"mode":"HubAndSpoke","hubs":["worker-1"]}}}' +``` +**Expect:** FullMesh → 6 gateways, flag cleared on all; HubAndSpoke → 4 +gateways, spoke↔spoke removed, spoke flags reconciled back to `true`. Verified. + +### E9 — Hub change round-trip (the flag-fix scenario) +```bash +kubectl patch sliceconfig demo-hub-and-spoke -n kubeslice-avesha --context kind-kubeslice-controller --type=merge -p='{"spec":{"topology":{"mode":"HubAndSpoke","hubs":["worker-2"]}}}' +kubectl patch sliceconfig demo-hub-and-spoke -n kubeslice-avesha --context kind-kubeslice-controller --type=merge -p='{"spec":{"topology":{"mode":"HubAndSpoke","hubs":["worker-1"]}}}' +``` +**Expect:** after the round-trip, **no `Server` gateway carries +`routeEntireSliceSubnet=true`** (the fix), and the dataplane recovers to 0% loss. +Verified. Note: a hub change is disruptive — gateway pods rebuild and the +dataplane takes several minutes to reconverge. + +### E10 — Add / remove a spoke +```bash +# remove worker-3 (update clusters AND applicationNamespaces together): +kubectl patch sliceconfig demo-hub-and-spoke -n kubeslice-avesha --context kind-kubeslice-controller --type=merge \ + -p='{"spec":{"clusters":["worker-1","worker-2"],"namespaceIsolationProfile":{"applicationNamespaces":[{"namespace":"iperf","clusters":["worker-1","worker-2"]}],"isolationEnabled":false}}}' +# add it back: same patch with worker-3 restored in both lists. +``` +**Expect:** remove → 2 gateways, worker-3 unreachable; add back → 4 gateways, +dataplane recovers to 0% loss (several-minute reconverge). Verified. + +### E11 — Webhook rejects invalid topologies +Apply the 7 invalid SliceConfigs (two hubs, hub-not-member, no-hubs, +hubs-without-mode, FullMesh-with-hubs, unknown-mode, duplicate-hubs). **Expect:** +each rejected at admission with the matching error from [Section 2](#2-controller-unit-tests-topology-webhook-validation). + +### E12 — Connection-status fields persist (#471) +```bash +kubectl patch workerslicegateway -n kubeslice-avesha --context kind-kubeslice-controller --subresource=status --type=merge \ + -p '{"status":{"connectionState":"Connected","reason":"TunnelEstablished","message":"up","lastTransitionTime":"2026-01-01T00:00:00Z"}}' +kubectl get workerslicegateway -n kubeslice-avesha --context kind-kubeslice-controller -o jsonpath='{.status.connectionState}|{.status.reason}|{.status.message}' +``` +**Expect:** the values persist. Before #471 the CRD lacked these fields and the +API server pruned them. + +--- + +## 6. Coverage map (issue → tests) + +| Issue / PR | Scope | Covered by | +|---|---|---| +| #300 / #406 | ADR: partial-mesh MVP design | design doc | +| #301 / #408 | Topology API + webhook | Section 2 (`test_validateTopology`, 10 cases) | +| #302 / #410 | Edge computation | Section 1 (`TestResolveTopologyEdges`, `TestTopologyEdgeSetContains`) | +| #303 / #422 | Gateway health → `TopologyConverged` status | Section 3a (`TestBuildTopologyConvergedCondition`, 5 cases) | +| #304 / #424 | Spoke gateways route entire slice + e2e | Section 3, Section 4, Section 4a, Section 4b; Section 5 | +| #471 / apis#44 / worker#495 | Gateway connection-status fields | Section 4c (worker derive/reconcile/reason); Section 5 E12 | +| worker #496 | Program entire-slice route (pod + router) | Section 4a; Section 5 E3–E5 | +| gateway-sidecar #64 | Route split + MSS clamp | Section 4b; Section 5 E3 | + +--- + +## 7. Testing notes & coverage gaps + +What could not be tested, and behaviour observed while running the suites above. + +- **WireGuard dataplane could not be tested (blocked, pre-existing).** The + control-plane *was* tested on WireGuard and passes — a `sliceGatewayType: + Wireguard` HubAndSpoke slice creates the correct gateways with the flag. But + the WireGuard **dataplane** cannot be exercised: the gateway pod gets stuck + `Init:0/1` with `references non-existent secret key: serverPublicKeyWgFile`, + because the controller has **no WireGuard key-generation code** (`GenerateCerts` + only produces the OpenVPN config, so the gateway secret contains + `ovpnConfigFile` instead of WireGuard keys). This is a pre-existing gap in the + controller's cert-generation subsystem, outside the spoke-to-spoke feature, so + every dataplane scenario in Section 5 is verified on **OpenVPN only**. Testing + WireGuard end-to-end first needs WireGuard key generation in the controller. +- **Observed: active-gateway failover is not instant (~15–20 s).** In scenario E6, + killing the *standby* gateway caused no disruption (0% loss); killing the *active* one + disrupted traffic for ~15–20 s while the slice router's ECMP withdrew the dead + next-hop, then recovered. Faster failover would need health-check-driven + next-hop withdrawal; the `/17` route split hardens the steady-state route but + does not speed up this reconvergence. +- **Out of scope: changing the hub on a live slice.** E9 verifies the + `RouteEntireSliceSubnet` flag reconciles correctly on a hub change (no stale + `true` on a server), but gateway VPN roles (`gatewayHostType`) are fixed at + creation and not re-assigned, so a live hub change leaves the old-hub↔new-hub + pair with stale roles. The hub is meant to be set at slice creation; to change + it, recreate the slice. +- **The feature ships as stacked PRs, so its tests live across several + branches.** No single branch has all of them checked out at once yet: + topology + edges + spoke routing + e2e (Sections 1–4b) are on the #304 branch; + `TopologyConverged` aggregation (Section 3a) is on the #303 branch (#422); the + worker connection-status tests (Section 4c) are on the worker + `feature/471-report-gateway-status` branch (#471/#495). They come together on + the `integration/hub-and-spoke-e2e` branch, which should be brought up to date + so `make test` / `make test-e2e-hns` run the whole suite in one place. This + document is the single reference for **all** of them regardless of branch. +- **The `service` unit tests need PR #404** to compile as a standalone test + binary (see [Layout](#layout)) — the one command in this doc that does not run + green today. diff --git a/test/e2e/harness_test.go b/test/e2e/harness_test.go new file mode 100644 index 00000000..ecab5fdc --- /dev/null +++ b/test/e2e/harness_test.go @@ -0,0 +1,234 @@ +//go:build e2e + +/* + * Copyright (c) 2026 Avesha, Inc. All rights reserved. + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Package e2e drives the Hub-and-Spoke (partial mesh) control-plane against a +// real, disposable Kind cluster running this branch's controller image. +// +// Scope is deliberately controller-focused: it runs the real controller +// (reconcilers + admission webhooks + CRDs) but stands in fake, pre-registered +// worker Cluster CRs instead of installing real worker-operators, NSM, or +// gateway pods. That makes the suite fast and deterministic and covers exactly +// where this feature's code lives — topology resolution, gateway edge creation, +// the RouteEntireSliceSubnet flag, and topology webhook validation. The +// dataplane (real tunnels, spoke-to-spoke traffic) is exercised by the manual +// runbook in docs/hub-and-spoke-testing.md, not here. +// +// The cluster name is prefixed e2e-hns- and created/destroyed within a single +// run, so it never touches a developer's own Kind clusters. Everything shells +// out to the kind/docker/kubectl/make CLIs. +package e2e + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +const ( + kindClusterName = "e2e-hns" + kubeContext = "kind-" + kindClusterName + controllerNS = "kubeslice-controller" + projectNS = "kubeslice-avesha" + sliceName = "e2e-hns-slice" +) + +// run executes a command, failing the test with combined output on error so a +// failure always shows what actually happened, not just "exit status 1". +func run(t *testing.T, dir, name string, args ...string) string { + t.Helper() + cmd := exec.Command(name, args...) + cmd.Dir = dir + var out bytes.Buffer + cmd.Stdout = &out + cmd.Stderr = &out + if err := cmd.Run(); err != nil { + t.Fatalf("%s %s: %v\n%s", name, strings.Join(args, " "), err, out.String()) + } + return out.String() +} + +// tryRun is like run but returns the combined output and success flag instead +// of failing the test — used for admission-webhook rejection checks, where a +// non-zero exit is the expected outcome. +func tryRun(name string, stdin string, args ...string) (string, bool) { + cmd := exec.Command(name, args...) + if stdin != "" { + cmd.Stdin = strings.NewReader(stdin) + } + var out bytes.Buffer + cmd.Stdout = &out + cmd.Stderr = &out + err := cmd.Run() + return out.String(), err == nil +} + +// kubectl runs kubectl against the e2e cluster. +func kubectl(t *testing.T, args ...string) string { + t.Helper() + return run(t, "", "kubectl", append([]string{"--context", kubeContext}, args...)...) +} + +// applyYAML pipes a manifest to `kubectl apply -f -`, failing on error. +func applyYAML(t *testing.T, yaml string) { + t.Helper() + cmd := exec.Command("kubectl", "--context", kubeContext, "apply", "-f", "-") + cmd.Stdin = strings.NewReader(yaml) + var out bytes.Buffer + cmd.Stdout = &out + cmd.Stderr = &out + if err := cmd.Run(); err != nil { + t.Fatalf("apply failed: %v\n%s\n---manifest---\n%s", err, out.String(), yaml) + } +} + +// tryApplyYAML pipes a manifest to apply and reports (output, accepted). Used +// for the invalid-topology webhook cases where rejection is expected. +func tryApplyYAML(yaml string) (string, bool) { + return tryRun("kubectl", yaml, "--context", kubeContext, "apply", "-f", "-") +} + +// waitFor polls until cond returns true or the timeout elapses. +func waitFor(t *testing.T, desc string, timeout time.Duration, cond func() bool) { + t.Helper() + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + if cond() { + return + } + time.Sleep(3 * time.Second) + } + t.Fatalf("timed out after %s waiting for: %s", timeout, desc) +} + +// repoRoot returns the controller repo root (two levels up from test/e2e). +func repoRoot(t *testing.T) string { + t.Helper() + wd, err := os.Getwd() + if err != nil { + t.Fatalf("getwd: %v", err) + } + return filepath.Clean(filepath.Join(wd, "..", "..")) +} + +// buildControllerImage builds this branch's controller image and returns its tag. +func buildControllerImage(t *testing.T) string { + t.Helper() + tag := fmt.Sprintf("kubeslice-controller-e2e:%d", time.Now().Unix()) + run(t, repoRoot(t), "docker", "build", "-t", tag, ".") + return tag +} + +// createCluster creates the disposable Kind cluster and registers teardown so +// every exit path (including t.Fatal in a later step) deletes it. +func createCluster(t *testing.T) { + t.Helper() + // clean any leftover from a previously killed run, then create fresh + _ = exec.Command("kind", "delete", "cluster", "--name", kindClusterName).Run() + run(t, "", "kind", "create", "cluster", "--name", kindClusterName, "--image", "kindest/node:v1.29.2") + t.Cleanup(func() { + _ = exec.Command("kind", "delete", "cluster", "--name", kindClusterName).Run() + }) +} + +// deployController installs cert-manager, loads the image, deploys the +// controller via `make deploy`, applies the two local-dev patches, and waits +// for the manager to be ready. +func deployController(t *testing.T, image string) { + t.Helper() + root := repoRoot(t) + + run(t, "", "kubectl", "--context", kubeContext, "apply", "-f", + "https://github.com/cert-manager/cert-manager/releases/download/v1.13.0/cert-manager.yaml") + run(t, "", "kubectl", "--context", kubeContext, "wait", "--for=condition=Available", + "deployment", "--all", "-n", "cert-manager", "--timeout=180s") + + run(t, "", "kind", "load", "docker-image", image, "--name", kindClusterName) + + // make deploy has no --context flag; it targets the current context. + run(t, "", "kubectl", "config", "use-context", kubeContext) + run(t, root, "make", "deploy", "IMG="+image) + + // local-dev patches (see docs/kubeslice-local-dev-setup): a reachable + // kube-rbac-proxy image and configmaps RBAC the reconciler needs. + _ = exec.Command("kubectl", "--context", kubeContext, "set", "image", + "deployment/kubeslice-controller-manager", + "kube-rbac-proxy=quay.io/brancz/kube-rbac-proxy:v0.8.0", "-n", controllerNS).Run() + _ = exec.Command("kubectl", "--context", kubeContext, "patch", "clusterrole", + "kubeslice-controller-controller-role", "--type=json", + "-p", `[{"op":"add","path":"/rules/-","value":{"apiGroups":[""],"resources":["configmaps"],"verbs":["get","list","watch"]}}]`).Run() + + run(t, "", "kubectl", "--context", kubeContext, "rollout", "status", + "deployment/kubeslice-controller-manager", "-n", controllerNS, "--timeout=180s") +} + +// setupProjectAndWorkers creates the project and three fake, pre-registered +// worker Cluster CRs (worker-1/2/3), standing in for a real registration flow. +func setupProjectAndWorkers(t *testing.T) { + t.Helper() + applyYAML(t, ` +apiVersion: controller.kubeslice.io/v1alpha1 +kind: Project +metadata: {name: avesha, namespace: `+controllerNS+`} +spec: {serviceAccount: {readWrite: [admin]}}`) + + waitFor(t, "project namespace "+projectNS+" exists", 60*time.Second, func() bool { + _, ok := tryRun("kubectl", "", "--context", kubeContext, "get", "ns", projectNS) + return ok + }) + + for _, n := range []string{"1", "2", "3"} { + applyYAML(t, ` +apiVersion: controller.kubeslice.io/v1alpha1 +kind: Cluster +metadata: {name: worker-`+n+`, namespace: `+projectNS+`} +spec: {networkInterface: eth0}`) + } + // patch status → Registered with a cniSubnet/nodeIP so the SliceConfig + // reconciler treats them as usable members. + time.Sleep(3 * time.Second) + for _, n := range []string{"1", "2", "3"} { + run(t, "", "kubectl", "--context", kubeContext, "patch", "cluster", "worker-"+n, + "-n", projectNS, "--subresource=status", "--type=merge", + "-p", `{"status":{"registrationStatus":"Registered","clusterHealth":{"clusterHealthStatus":"Normal"},"nodeIPs":["172.18.0.1`+n+`"],"networkPresent":true,"cniSubnet":["10.244.0.0/16"]}}`) + } +} + +// gatewayColumns returns "name host route" lines for the slice's gateways. +func gatewayColumns(t *testing.T) string { + t.Helper() + return kubectl(t, "get", "workerslicegateways", "-n", projectNS, + "-o", "custom-columns=N:.metadata.name,HOST:.spec.gatewayHostType,ROUTE:.spec.routeEntireSliceSubnet", + "--no-headers") +} + +// gatewayCount returns how many slice gateways currently exist. +func gatewayCount(t *testing.T) int { + t.Helper() + lines := strings.TrimSpace(gatewayColumns(t)) + if lines == "" { + return 0 + } + return len(strings.Split(lines, "\n")) +} diff --git a/test/e2e/hubandspoke_e2e_test.go b/test/e2e/hubandspoke_e2e_test.go new file mode 100644 index 00000000..6963664b --- /dev/null +++ b/test/e2e/hubandspoke_e2e_test.go @@ -0,0 +1,75 @@ +//go:build e2e + +/* + * Copyright (c) 2026 Avesha, Inc. All rights reserved. + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package e2e + +import ( + "testing" + "time" +) + +// TestHubAndSpokeE2E stands up one disposable Kind cluster with this branch's +// controller, three fake registered workers, and runs the Hub-and-Spoke +// control-plane scenarios against it in order. The heavy setup (image build + +// cluster + controller deploy) happens once; each scenario is a subtest. +func TestHubAndSpokeE2E(t *testing.T) { + image := buildControllerImage(t) + createCluster(t) + deployController(t, image) + setupProjectAndWorkers(t) + + // Baseline must run first: it applies the slice the later scenarios mutate. + t.Run("PartialMesh_HubAndSpokeSkipsSpokeToSpoke", scenarioPartialMesh) + t.Run("FullMesh_Unaffected", scenarioFullMeshUnaffected) + t.Run("TopologyChange_ReconcilesFlag", scenarioTopologyChangeReconcilesFlag) + t.Run("HubChange_NoStaleServerFlag", scenarioHubChangeNoStaleFlag) + t.Run("Webhook_RejectsInvalidTopologies", scenarioWebhookRejectsInvalid) + t.Run("StatusFields_Persist", scenarioStatusFieldsPersist) +} + +// sliceManifest builds a SliceConfig manifest with the given topology block. +// topology is the YAML for the spec.topology field (or "" to omit it). +func sliceManifest(topology string) string { + m := ` +apiVersion: controller.kubeslice.io/v1alpha1 +kind: SliceConfig +metadata: {name: ` + sliceName + `, namespace: ` + projectNS + `} +spec: + sliceType: Application + sliceSubnet: 10.11.0.0/16 + sliceGatewayProvider: {sliceGatewayType: OpenVPN, sliceCaType: Local} + sliceIpamType: Local + clusters: [worker-1, worker-2, worker-3] +` + if topology != "" { + m += " " + topology + "\n" + } + m += ` qosProfileDetails: {queueType: HTB, priority: 1, tcType: BANDWIDTH_CONTROL, bandwidthCeilingKbps: 5120, bandwidthGuaranteedKbps: 2560, dscpClass: AF11} + namespaceIsolationProfile: {isolationEnabled: false}` + return m +} + +// waitForGatewayCount blocks until exactly n slice gateways exist. +func waitForGatewayCount(t *testing.T, n int) { + t.Helper() + waitFor(t, "slice has exactly gateways", 60*time.Second, func() bool { + return gatewayCount(t) == n + }) +} diff --git a/test/e2e/scenarios_test.go b/test/e2e/scenarios_test.go new file mode 100644 index 00000000..bbef3ad9 --- /dev/null +++ b/test/e2e/scenarios_test.go @@ -0,0 +1,183 @@ +//go:build e2e + +/* + * Copyright (c) 2026 Avesha, Inc. All rights reserved. + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package e2e + +import ( + "strings" + "testing" + "time" +) + +// gw returns the fully-qualified gateway object name for a source→dest pair. +func gw(source, dest string) string { return sliceName + "-worker-" + source + "-worker-" + dest } + +// gatewayRow returns the (host, route) columns for a gateway, or ("","") if absent. +func gatewayRow(t *testing.T, name string) (host, route string) { + t.Helper() + for _, line := range strings.Split(strings.TrimSpace(gatewayColumns(t)), "\n") { + f := strings.Fields(line) + if len(f) == 3 && f[0] == name { + return f[1], f[2] + } + } + return "", "" +} + +func assertGateway(t *testing.T, name, wantHost, wantRoute string) { + t.Helper() + host, route := gatewayRow(t, name) + if host == "" { + t.Fatalf("gateway %s does not exist (expected host=%s route=%s)", name, wantHost, wantRoute) + } + if host != wantHost || route != wantRoute { + t.Fatalf("gateway %s: got host=%s route=%s, want host=%s route=%s", name, host, route, wantHost, wantRoute) + } +} + +func assertNoGateway(t *testing.T, name string) { + t.Helper() + if host, _ := gatewayRow(t, name); host != "" { + t.Fatalf("gateway %s exists but should not", name) + } +} + +// serversWithRoute counts gateways that are Server-side AND carry route=true — +// which must always be zero (a hub/server never routes the whole slice). +func serversWithRoute(t *testing.T) int { + t.Helper() + n := 0 + for _, line := range strings.Split(strings.TrimSpace(gatewayColumns(t)), "\n") { + f := strings.Fields(line) + if len(f) == 3 && f[1] == "Server" && f[2] == "true" { + n++ + } + } + return n +} + +// scenarioPartialMesh applies a HubAndSpoke slice (hub=worker-1) and asserts the +// controller builds only the two hub↔spoke pairs, with the spoke side flagged +// and no spoke↔spoke gateway. +func scenarioPartialMesh(t *testing.T) { + applyYAML(t, sliceManifest("topology: {mode: HubAndSpoke, hubs: [worker-1]}")) + waitForGatewayCount(t, 4) + assertGateway(t, gw("1", "2"), "Server", "") + assertGateway(t, gw("1", "3"), "Server", "") + assertGateway(t, gw("2", "1"), "Client", "true") + assertGateway(t, gw("3", "1"), "Client", "true") + assertNoGateway(t, gw("2", "3")) + assertNoGateway(t, gw("3", "2")) +} + +// scenarioFullMeshUnaffected switches the slice to full mesh and asserts every +// pair is built with the flag off — the backward-compatibility guarantee. +func scenarioFullMeshUnaffected(t *testing.T) { + run(t, "", "kubectl", "--context", kubeContext, "patch", "sliceconfig", sliceName, + "-n", projectNS, "--type=json", "-p", `[{"op":"remove","path":"/spec/topology"}]`) + waitForGatewayCount(t, 6) + if n := serversWithRoute(t); n != 0 { + t.Fatalf("full mesh: %d server gateways carry route=true, want 0", n) + } + // no gateway of any role should carry the flag in full mesh + for _, line := range strings.Split(strings.TrimSpace(gatewayColumns(t)), "\n") { + if f := strings.Fields(line); len(f) == 3 && f[2] == "true" { + t.Fatalf("full mesh: gateway %s unexpectedly carries route=true", f[0]) + } + } +} + +// scenarioTopologyChangeReconcilesFlag switches back to HubAndSpoke and asserts +// the surviving spoke gateways get RouteEntireSliceSubnet reconciled to true +// (a FullMesh→HubAndSpoke switch must not leave the flag stale-false). +func scenarioTopologyChangeReconcilesFlag(t *testing.T) { + run(t, "", "kubectl", "--context", kubeContext, "patch", "sliceconfig", sliceName, + "-n", projectNS, "--type=merge", + "-p", `{"spec":{"topology":{"mode":"HubAndSpoke","hubs":["worker-1"]}}}`) + waitForGatewayCount(t, 4) + assertGateway(t, gw("2", "1"), "Client", "true") + assertGateway(t, gw("3", "1"), "Client", "true") + assertNoGateway(t, gw("2", "3")) +} + +// scenarioHubChangeNoStaleFlag does a hub-change round-trip (worker-1 → worker-2 +// → worker-1) and asserts no server gateway is left carrying route=true — the +// regression fixed by reconciling the flag on both sides of an existing pair. +func scenarioHubChangeNoStaleFlag(t *testing.T) { + run(t, "", "kubectl", "--context", kubeContext, "patch", "sliceconfig", sliceName, + "-n", projectNS, "--type=merge", + "-p", `{"spec":{"topology":{"mode":"HubAndSpoke","hubs":["worker-2"]}}}`) + time.Sleep(6 * time.Second) + run(t, "", "kubectl", "--context", kubeContext, "patch", "sliceconfig", sliceName, + "-n", projectNS, "--type=merge", + "-p", `{"spec":{"topology":{"mode":"HubAndSpoke","hubs":["worker-1"]}}}`) + // give the reconciler time to settle back to hub=worker-1 + waitFor(t, "hub-change round-trip leaves no server with route=true", 60*time.Second, func() bool { + return gatewayCount(t) == 4 && serversWithRoute(t) == 0 + }) + assertGateway(t, gw("1", "2"), "Server", "") + assertGateway(t, gw("2", "1"), "Client", "true") +} + +// scenarioWebhookRejectsInvalid asserts the admission webhook (and CRD schema) +// reject every malformed topology. +func scenarioWebhookRejectsInvalid(t *testing.T) { + cases := []struct { + name string + topology string + }{ + {"two hubs", "topology: {mode: HubAndSpoke, hubs: [worker-1, worker-2]}"}, + {"hub not a member", "topology: {mode: HubAndSpoke, hubs: [worker-9]}"}, + {"no hubs", "topology: {mode: HubAndSpoke, hubs: []}"}, + {"hubs without mode", "topology: {hubs: [worker-1]}"}, + {"FullMesh with hubs", "topology: {mode: FullMesh, hubs: [worker-1]}"}, + {"unknown mode", "topology: {mode: Banana, hubs: [worker-1]}"}, + {"duplicate hubs", "topology: {mode: HubAndSpoke, hubs: [worker-1, worker-1]}"}, + } + for _, c := range cases { + // use a distinct name per case so a stray accepted object is obvious + manifest := strings.Replace(sliceManifest(c.topology), sliceName, sliceName+"-bad", 1) + out, accepted := tryApplyYAML(manifest) + if accepted { + // clean up the wrongly-accepted object before failing + _, _ = tryRun("kubectl", "", "--context", kubeContext, "delete", "sliceconfig", sliceName+"-bad", "-n", projectNS) + t.Fatalf("invalid topology %q was accepted, expected rejection", c.name) + } + if !strings.Contains(strings.ToLower(out), "invalid") && !strings.Contains(out, "Unsupported") && !strings.Contains(out, "Too many") { + t.Fatalf("invalid topology %q rejected without a clear error:\n%s", c.name, out) + } + } +} + +// scenarioStatusFieldsPersist patches the #471 connection-status fields on a +// gateway and asserts they survive (before #471 the CRD lacked them and the API +// server pruned them). +func scenarioStatusFieldsPersist(t *testing.T) { + name := gw("2", "1") + run(t, "", "kubectl", "--context", kubeContext, "patch", "workerslicegateway", name, + "-n", projectNS, "--subresource=status", "--type=merge", + "-p", `{"status":{"connectionState":"Connected","reason":"TunnelEstablished","message":"up","lastTransitionTime":"2026-01-01T00:00:00Z"}}`) + got := strings.TrimSpace(kubectl(t, "get", "workerslicegateway", name, "-n", projectNS, + "-o", `jsonpath={.status.connectionState}|{.status.reason}|{.status.message}`)) + want := "Connected|TunnelEstablished|up" + if got != want { + t.Fatalf("status fields did not persist: got %q, want %q", got, want) + } +} From 3065d34055309a01d6debb0565825ab26f934fdc Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Fri, 28 Aug 2026 18:15:44 +0530 Subject: [PATCH 16/21] address review: correct hub-and-spoke sample note and enqueue SliceConfig only on gateway connection-state change Signed-off-by: Shreesha001 --- ...er_v1alpha1_sliceconfig_hub_and_spoke.yaml | 8 +++---- .../controller/sliceconfig_controller.go | 23 ++++++++++++++++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml b/config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml index 5e1ccec2..2eb6c766 100644 --- a/config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml +++ b/config/samples/controller_v1alpha1_sliceconfig_hub_and_spoke.yaml @@ -3,11 +3,9 @@ # worker-1 acts as the hub; worker-2 and worker-3 become spokes (all # non-hub members are spokes). # -# Note: this sample exercises the API and validation only. As of this -# release the controller does not yet consume spec.topology to change -# gateway/peer link creation (that is follow-up implementation work); -# the intended result once it does is hub<->spoke links only -# (worker-1<->worker-2, worker-1<->worker-3) and no spoke<->spoke link. +# The controller consumes spec.topology for this sample and creates +# hub<->spoke links only: worker-1<->worker-2 and worker-1<->worker-3. +# No spoke<->spoke link is created. # # Omitting spec.topology entirely (or setting mode: FullMesh with no # hubs) keeps the existing full-mesh behavior. diff --git a/controllers/controller/sliceconfig_controller.go b/controllers/controller/sliceconfig_controller.go index c658c1e2..be7f9dfb 100644 --- a/controllers/controller/sliceconfig_controller.go +++ b/controllers/controller/sliceconfig_controller.go @@ -30,8 +30,11 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" ) // SliceConfigReconciler reconciles a SliceConfig object @@ -64,10 +67,28 @@ func (r *SliceConfigReconciler) sliceConfigForGateway(ctx context.Context, obj c } } +// gatewayConnectionStateChanged only lets a WorkerSliceGateway event through when +// it can actually affect the slice's TopologyConverged condition: any create or +// delete, or an update that changes the gateway's status.ConnectionState. This +// filters out the frequent status writes that don't move connectivity (latency, +// rates, message/reason churn) so large or noisy slices don't trigger a +// SliceConfig reconcile on every gateway heartbeat. +var gatewayConnectionStateChanged = predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + oldGw, ok1 := e.ObjectOld.(*workerv1alpha1.WorkerSliceGateway) + newGw, ok2 := e.ObjectNew.(*workerv1alpha1.WorkerSliceGateway) + if !ok1 || !ok2 { + return true + } + return oldGw.Status.ConnectionState != newGw.Status.ConnectionState + }, +} + // SetupWithManager sets up the controller with the Manager. func (r *SliceConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&controllerv1alpha1.SliceConfig{}). - Watches(&workerv1alpha1.WorkerSliceGateway{}, handler.EnqueueRequestsFromMapFunc(r.sliceConfigForGateway)). + Watches(&workerv1alpha1.WorkerSliceGateway{}, handler.EnqueueRequestsFromMapFunc(r.sliceConfigForGateway), + builder.WithPredicates(gatewayConnectionStateChanged)). Complete(r) } From b2a5d152db11021041db68c147720a21181d5f53 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Fri, 28 Aug 2026 18:19:48 +0530 Subject: [PATCH 17/21] address review: reconcile TopologyConverged for no-network slices and remove dead require.NoError(t, nil) assertions Signed-off-by: Shreesha001 --- service/slice_config_service.go | 12 ++++++++++-- service/worker_slice_gateway_service_test.go | 9 --------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/service/slice_config_service.go b/service/slice_config_service.go index e6316168..d4528dc1 100644 --- a/service/slice_config_service.go +++ b/service/slice_config_service.go @@ -188,8 +188,16 @@ func (s *SliceConfigService) ReconcileSliceConfig(ctx context.Context, req ctrl. ownershipLabel := util.GetOwnerLabel(completeResourceName) if sliceConfig.Spec.OverlayNetworkDeploymentMode == v1alpha1.NONET { - err = s.ms.CreateMinimalWorkerSliceConfigForNoNetworkSlice(ctx, sliceConfig.Spec.Clusters, req.Namespace, ownershipLabel, sliceConfig.Name) - return ctrl.Result{}, err + if err = s.ms.CreateMinimalWorkerSliceConfigForNoNetworkSlice(ctx, sliceConfig.Spec.Clusters, req.Namespace, ownershipLabel, sliceConfig.Name); err != nil { + return ctrl.Result{}, err + } + // A no-network slice has no gateway links, so its topology is trivially + // converged (TopologyConverged=True, reason NoGatewaysRequired). Reconcile + // it here since this path returns before Step 9. + if err := s.reconcileTopologyStatus(ctx, sliceConfig, req.Namespace, ownershipLabel); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } // Step 4: Creation of worker slice Objects and Cluster Labels diff --git a/service/worker_slice_gateway_service_test.go b/service/worker_slice_gateway_service_test.go index 5b2ca81e..8e794cb2 100644 --- a/service/worker_slice_gateway_service_test.go +++ b/service/worker_slice_gateway_service_test.go @@ -110,7 +110,6 @@ func testWorkerSliceGatewayReconciliationSuccess(t *testing.T) { clientMock.On("Update", ctx, mock.Anything).Return(nil).Once() result, err := workerSliceGatewayService.ReconcileWorkerSliceGateways(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, result, expectedResult) require.Nil(t, err) clientMock.AssertExpectations(t) @@ -151,7 +150,6 @@ func testWorkerSliceGatewayReconciliationIfSliceConfigNotFound(t *testing.T) { clientMock.On("Get", ctx, mock.AnythingOfType("types.NamespacedName"), sliceConfig).Return(notFoundError).Once() result, err := workerSliceGatewayService.ReconcileWorkerSliceGateways(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, result, expectedResult) require.Nil(t, err) clientMock.AssertExpectations(t) @@ -164,7 +162,6 @@ func testWorkerSliceGatewayReconciliationIfGatewayNotFound(t *testing.T) { clientMock.On("Get", ctx, requestObj.NamespacedName, WorkerSliceGateway).Return(notFoundError).Once() result, err := workerSliceGatewayService.ReconcileWorkerSliceGateways(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, result, expectedResult) require.Nil(t, err) clientMock.AssertExpectations(t) @@ -187,7 +184,6 @@ func testWorkerSliceGatewayReconciliationDelete(t *testing.T) { clientMock.On("Get", ctx, mock.AnythingOfType("types.NamespacedName"), sliceConfig).Return(notFoundError).Once() result, err := workerSliceGatewayService.ReconcileWorkerSliceGateways(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, result, expectedResult) require.Nil(t, err) clientMock.AssertExpectations(t) @@ -242,7 +238,6 @@ func testWorkerSliceGatewayReconciliationDeleteForcefully(t *testing.T) { clientMock.On("Update", ctx, mock.Anything).Return(nil).Once() result, err := workerSliceGatewayService.ReconcileWorkerSliceGateways(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, result, expectedResult) require.Nil(t, err) clientMock.AssertExpectations(t) @@ -324,7 +319,6 @@ func testCreateMinimumWorkerSliceGatewaysAlreadyExists(t *testing.T) { result, err := workerSliceGatewayService.CreateMinimumWorkerSliceGateways(ctx, "red", clusterNames, requestObj.Namespace, label, clusterMap, "10.10.10.10/16", "/16", nil, nil) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, result, expectedResult) require.Nil(t, err) clientMock.AssertExpectations(t) @@ -526,7 +520,6 @@ func testCreateMinimumWorkerSliceGatewaysNotExists(t *testing.T) { 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", nil, nil) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, result, expectedResult) require.Nil(t, err) clientMock.AssertExpectations(t) @@ -594,7 +587,6 @@ func testDeleteWorkerSliceGatewaysByLabelExists(t *testing.T) { clientMock.On("Update", ctx, mock.AnythingOfType("*v1.Event")).Return(nil).Once() mMock.On("RecordCounterMetric", mock.Anything, mock.Anything).Return().Once() err := workerSliceGatewayService.DeleteWorkerSliceGatewaysByLabel(ctx, label, "namespace") - require.NoError(t, nil) require.Nil(t, err) clientMock.AssertExpectations(t) mMock.AssertExpectations(t) @@ -675,7 +667,6 @@ func testNodeIpReconciliationOfWorkerSliceGatewaysExists(t *testing.T) { Status: controllerv1alpha1.ClusterStatus{}, } err := workerSliceGatewayService.NodeIpReconciliationOfWorkerSliceGateways(ctx, &cluster, "namespace") - require.NoError(t, nil) require.Nil(t, err) clientMock.AssertExpectations(t) } From fa3ea3c09e0574643f3645b332da9942752a36c1 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Fri, 28 Aug 2026 20:22:10 +0530 Subject: [PATCH 18/21] address review: reject HubAndSpoke on no-network slices, add hub-change and NONET status tests, remove dead assertions, document reconcile cost Signed-off-by: Shreesha001 --- .../controller/sliceconfig_controller.go | 6 + .../sliceconfig_hubandspoke_test.go | 103 ++++++++++++++++++ service/slice_config_service.go | 5 + service/slice_config_service_test.go | 8 -- service/slice_config_webhook_validation.go | 6 + .../slice_config_webhook_validation_test.go | 14 ++- 6 files changed, 132 insertions(+), 10 deletions(-) diff --git a/controllers/controller/sliceconfig_controller.go b/controllers/controller/sliceconfig_controller.go index be7f9dfb..d493303f 100644 --- a/controllers/controller/sliceconfig_controller.go +++ b/controllers/controller/sliceconfig_controller.go @@ -85,6 +85,12 @@ var gatewayConnectionStateChanged = predicate.Funcs{ } // SetupWithManager sets up the controller with the Manager. +// +// A gateway connection-state change enqueues the owning SliceConfig, which runs +// the full reconcile (it recomputes the whole desired state, then the +// TopologyConverged status). The gatewayConnectionStateChanged predicate keeps +// this to real Connected/NotConnected transitions, so the full-reconcile cost is +// paid only on genuine convergence changes, not on every status heartbeat. func (r *SliceConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&controllerv1alpha1.SliceConfig{}). diff --git a/controllers/controller/sliceconfig_hubandspoke_test.go b/controllers/controller/sliceconfig_hubandspoke_test.go index bf9fef95..48b66015 100644 --- a/controllers/controller/sliceconfig_hubandspoke_test.go +++ b/controllers/controller/sliceconfig_hubandspoke_test.go @@ -10,6 +10,7 @@ import ( . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -270,4 +271,106 @@ var _ = Describe("SliceConfig HubAndSpoke topology (partial mesh)", Ordered, fun _ = k8sClient.Delete(ctx, latest) } }) + + It("clears the flag on a gateway that becomes the hub side after a hub change", func() { + // Directly exercises the both-sides RouteEntireSliceSubnet reconcile across a + // hub change (the higher-risk transition the code comment calls out). With + // hub=worker-1, the gateway -worker-2-worker-1 is the spoke's client + // (flag true). Changing the hub to worker-2 makes that same gateway the new + // hub's server side, so its flag MUST be cleared to false - otherwise the hub + // would route the entire slice and misdirect traffic. Symmetrically, + // -worker-1-worker-2 flips from hub server (false) to spoke client (true). + const hcName = "hubchange-slice" + key := types.NamespacedName{Name: hcName, Namespace: nsName} + becomesServer := hcName + "-worker-2-worker-1" // client(true) now -> server(false) after change + becomesClient := hcName + "-worker-1-worker-2" // server(false) now -> client(true) after change + + slice := &v1alpha1.SliceConfig{ + ObjectMeta: metav1.ObjectMeta{Name: hcName, Namespace: nsName}, + Spec: v1alpha1.SliceConfigSpec{ + Clusters: []string{"worker-1", "worker-2", "worker-3"}, + MaxClusters: 4, + SliceSubnet: "10.12.0.0/16", + SliceGatewayProvider: &v1alpha1.WorkerSliceGatewayProvider{ + SliceGatewayType: "OpenVPN", + SliceCaType: "Local", + }, + SliceIpamType: "Local", + SliceType: "Application", + Topology: &v1alpha1.TopologySpec{Mode: v1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"worker-1"}}, + QosProfileDetails: &v1alpha1.QOSProfile{ + BandwidthCeilingKbps: 5120, + DscpClass: "AF11", + }, + }, + } + Expect(k8sClient.Create(ctx, slice)).Should(Succeed()) + + // initial state (hub=worker-1) + Eventually(func() bool { return gatewayExists(becomesServer) && gatewayExists(becomesClient) }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return getGateway(becomesServer).Spec.RouteEntireSliceSubnet }, timeout, interval).Should(BeTrue()) + Expect(getGateway(becomesClient).Spec.RouteEntireSliceSubnet).To(BeFalse()) + + // change the hub to worker-2 + latest := &v1alpha1.SliceConfig{} + Expect(k8sClient.Get(ctx, key, latest)).Should(Succeed()) + latest.Spec.Topology = &v1alpha1.TopologySpec{Mode: v1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"worker-2"}} + Expect(k8sClient.Update(ctx, latest)).Should(Succeed()) + + // the former client (now the hub server side) must be cleared to false + Eventually(func() bool { return getGateway(becomesServer).Spec.RouteEntireSliceSubnet }, timeout, interval).Should(BeFalse()) + // and the former server (now the spoke client side) must be set to true + Eventually(func() bool { return getGateway(becomesClient).Spec.RouteEntireSliceSubnet }, timeout, interval).Should(BeTrue()) + + // cleanup (best-effort) + if k8sClient.Get(ctx, key, latest) == nil { + latest.Spec.Clusters = []string{} + _ = k8sClient.Update(ctx, latest) + _ = k8sClient.Delete(ctx, latest) + } + }) + + It("marks a no-network slice TopologyConverged=True with NoGatewaysRequired", func() { + // A no-network (NONET) slice has no gateway links, so it must still report a + // TopologyConverged condition (True / NoGatewaysRequired). This guards the + // early-return path that previously skipped the status write entirely. + const nnName = "nonet-slice" + slice := &v1alpha1.SliceConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nnName, Namespace: nsName}, + Spec: v1alpha1.SliceConfigSpec{ + Clusters: []string{"worker-1", "worker-2"}, + MaxClusters: 4, + SliceSubnet: "10.13.0.0/16", + OverlayNetworkDeploymentMode: v1alpha1.NONET, + SliceGatewayProvider: &v1alpha1.WorkerSliceGatewayProvider{ + SliceGatewayType: "OpenVPN", + SliceCaType: "Local", + }, + SliceIpamType: "Local", + SliceType: "Application", + QosProfileDetails: &v1alpha1.QOSProfile{ + BandwidthCeilingKbps: 5120, + DscpClass: "AF11", + }, + }, + } + Expect(k8sClient.Create(ctx, slice)).Should(Succeed()) + + Eventually(func() bool { + s := v1alpha1.SliceConfig{} + if k8sClient.Get(ctx, types.NamespacedName{Name: nnName, Namespace: nsName}, &s) != nil { + return false + } + cond := apimeta.FindStatusCondition(s.Status.Conditions, v1alpha1.SliceConditionTypeTopologyConverged) + return cond != nil && cond.Status == metav1.ConditionTrue && cond.Reason == v1alpha1.SliceReasonNoGatewaysRequired + }, timeout, interval).Should(BeTrue()) + + // cleanup (best-effort) + nn := v1alpha1.SliceConfig{} + if k8sClient.Get(ctx, types.NamespacedName{Name: nnName, Namespace: nsName}, &nn) == nil { + nn.Spec.Clusters = []string{} + _ = k8sClient.Update(ctx, &nn) + _ = k8sClient.Delete(ctx, &nn) + } + }) }) diff --git a/service/slice_config_service.go b/service/slice_config_service.go index d4528dc1..a9f0b2af 100644 --- a/service/slice_config_service.go +++ b/service/slice_config_service.go @@ -261,6 +261,11 @@ func (s *SliceConfigService) ReconcileSliceConfig(ctx context.Context, req ctrl. // and persists it only when the condition changed (so LastTransitionTime and the // status subresource are not churned on every reconcile). func (s *SliceConfigService) reconcileTopologyStatus(ctx context.Context, sliceConfig *v1alpha1.SliceConfig, namespace string, ownershipLabel map[string]string) error { + // The gateway list is read once outside the retry loop below. On a write + // conflict the retry reuses this snapshot, which is benign: any gateway + // connectivity change also enqueues a fresh SliceConfig reconcile (via the + // WorkerSliceGateway watch), so a slightly stale list self-corrects on the + // next pass rather than needing a re-list here. gateways, err := s.sgs.ListWorkerSliceGateways(ctx, ownershipLabel, namespace) if err != nil { return err diff --git a/service/slice_config_service_test.go b/service/slice_config_service_test.go index cceb0900..bc1aaddc 100644 --- a/service/slice_config_service_test.go +++ b/service/slice_config_service_test.go @@ -146,7 +146,6 @@ func SliceConfigReconciliationCompleteHappyCase(t *testing.T) { clientMock.On("Status").Return(&fakeStatusWriter{}) result, err := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, expectedResult, result) require.Nil(t, err) require.False(t, result.Requeue) @@ -181,7 +180,6 @@ func SliceConfigReconciliationNoNetCompleteHappyCase(t *testing.T) { result, err := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, expectedResult, result) require.Nil(t, err) require.False(t, result.Requeue) @@ -209,7 +207,6 @@ func SliceConfigGetObjectErrorNotFound(t *testing.T) { clientMock.On("Get", ctx, requestObj.NamespacedName, sliceConfig).Return(notFoundError).Once() result, err2 := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, expectedResult, result) require.Nil(t, err2) require.False(t, result.Requeue) @@ -237,7 +234,6 @@ func SliceConfigDeleteTheObjectHappyCase(t *testing.T) { mMock.On("RecordCounterMetric", mock.Anything, mock.Anything).Return().Once() result, err := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, expectedResult, result) require.Nil(t, err) require.False(t, result.Requeue) @@ -266,7 +262,6 @@ func SliceConfigObjectNamespaceNotFound(t *testing.T) { }).Once() result, err := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, expectedResult, result) require.Nil(t, err) require.False(t, result.Requeue) @@ -291,7 +286,6 @@ func SliceConfigObjectNotInProjectNamespace(t *testing.T) { }).Once() result, err := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, expectedResult, result) require.Nil(t, err) require.False(t, result.Requeue) @@ -311,7 +305,6 @@ func SliceConfigObjectWithDuplicateClustersInSpec(t *testing.T) { }).Once() result, err := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, expectedResult, result) require.Nil(t, err) require.False(t, result.Requeue) @@ -566,7 +559,6 @@ func SliceConfigDeleteHappyCase(t *testing.T) { mMock.On("RecordCounterMetric", mock.Anything, mock.Anything).Return().Once() result, err := sliceConfigService.DeleteSliceConfigs(ctx, requestObj.Namespace) expectedResult := ctrl.Result{} - require.NoError(t, nil) require.Equal(t, expectedResult, result) require.Nil(t, err) clientMock.AssertExpectations(t) diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index de457f76..41311452 100644 --- a/service/slice_config_webhook_validation.go +++ b/service/slice_config_webhook_validation.go @@ -365,6 +365,12 @@ func validateTopology(sliceConfig *controllerv1alpha1.SliceConfig) *field.Error // validateHubAndSpokeTopology is function to validate the hub and spoke topology rules func validateHubAndSpokeTopology(sliceConfig *controllerv1alpha1.SliceConfig, topologyPath *field.Path) *field.Error { topology := sliceConfig.Spec.Topology + // A no-network slice has no gateways, so the controller ignores spec.topology + // entirely for it. Reject HubAndSpoke here rather than silently doing nothing, + // so the user isn't misled into thinking they configured a partial mesh. + if sliceConfig.Spec.OverlayNetworkDeploymentMode == controllerv1alpha1.NONET { + return field.Invalid(topologyPath.Child("Mode"), string(topology.Mode), "HubAndSpoke topology is not supported for a no-network slice (overlayNetworkDeploymentMode=no-network has no gateway links)") + } if len(sliceConfig.Spec.Clusters) < 2 { return field.Invalid(topologyPath.Child("Mode"), string(topology.Mode), "HubAndSpoke topology requires at least 2 clusters") } diff --git a/service/slice_config_webhook_validation_test.go b/service/slice_config_webhook_validation_test.go index 92fc3b34..ff00fb2e 100644 --- a/service/slice_config_webhook_validation_test.go +++ b/service/slice_config_webhook_validation_test.go @@ -2324,6 +2324,7 @@ func test_validateTopology(t *testing.T) { name string clusters []string topology *controllerv1alpha1.TopologySpec + overlayMode controllerv1alpha1.NetworkType wantErr bool errContains string }{ @@ -2394,13 +2395,22 @@ func test_validateTopology(t *testing.T) { wantErr: true, errContains: "mode must be set to HubAndSpoke when hubs is specified", }, + { + name: "HubAndSpoke on a no-network slice is rejected", + clusters: clusters, + topology: &controllerv1alpha1.TopologySpec{Mode: controllerv1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"cluster-1"}}, + overlayMode: controllerv1alpha1.NONET, + wantErr: true, + errContains: "not supported for a no-network slice", + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { sliceConfig := &controllerv1alpha1.SliceConfig{ Spec: controllerv1alpha1.SliceConfigSpec{ - Clusters: tc.clusters, - Topology: tc.topology, + Clusters: tc.clusters, + Topology: tc.topology, + OverlayNetworkDeploymentMode: tc.overlayMode, }, } err := validateTopology(sliceConfig) From e45597c6cbb2e9e8c36affa8c2aa73636b541488 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Fri, 28 Aug 2026 20:25:01 +0530 Subject: [PATCH 19/21] docs: update hub-and-spoke test counts for new webhook, derive, route-teardown and integration tests Signed-off-by: Shreesha001 --- docs/hub-and-spoke-testing.md | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/docs/hub-and-spoke-testing.md b/docs/hub-and-spoke-testing.md index 9319b8f0..912063a6 100644 --- a/docs/hub-and-spoke-testing.md +++ b/docs/hub-and-spoke-testing.md @@ -77,7 +77,7 @@ cd worker-operator && go test ./pkg/hub/controllers/ -run 'TestNewMeshGatewayCon cd worker-operator && go test ./pkg/hub/controllers/ -run 'TestDeriveGatewayConnectionState|TestReconcileGatewayConnectionStatus|TestReasonMessageForState' # gateway-sidecar route split + MSS clamp -cd gateway-sidecar && go test ./pkg/sidecar/sidecarpb/ -run 'TestMoreSpecificHalves|TestTunnelMSSClampCommands' +cd gateway-sidecar && go test ./pkg/sidecar/sidecarpb/ -run 'TestMoreSpecificHalves|TestTunnelMSSClampCommands|TestStaleTunnelRouteKeys' ``` > **Known gate — the controller `service` package.** On `master` today the @@ -138,7 +138,7 @@ File: `service/slice_config_webhook_validation_test.go` → `test_validateTopolo Wired into **both** `ValidateSliceConfigCreate` and `ValidateSliceConfigUpdate`, so a topology change on a live slice is validated too. -10 table-driven cases — 3 accepted, 7 rejected (with the exact error text +11 table-driven cases — 3 accepted, 8 rejected (with the exact error text asserted): | # | Case | Result | Error contains | @@ -153,6 +153,7 @@ asserted): | 8 | more than one hub (single-hub MVP) | reject | `only one hub is supported in this release` | | 9 | unknown mode | reject | `unknown topology mode` | | 10 | hubs without mode | reject | `mode must be set to HubAndSpoke when hubs is specified` | +| 11 | HubAndSpoke on a no-network slice | reject | `not supported for a no-network slice` | Note: the single-hub restriction (case 8) is ordered **before** the duplicate check, so `[worker-1, worker-1]` reports "only one hub is supported" (the more @@ -205,7 +206,7 @@ single `TopologyConverged` condition on the SliceConfig. 5 cases: ## 4. Controller reconciler tests (envtest) File: `controllers/controller/sliceconfig_hubandspoke_test.go` (Ginkgo, real API -server via envtest). 3 specs: +server via envtest). 5 specs: - **`builds only hub<->spoke gateways and skips spoke<->spoke`** — applying a HubAndSpoke SliceConfig creates the 4 WorkerSliceGateway objects (2 pairs) and @@ -216,6 +217,14 @@ server via envtest). 3 specs: - **`reconciles RouteEntireSliceSubnet when the topology changes on an existing slice`** — switching an existing slice's topology updates the flag on the surviving gateways (the control-plane half of the E8/E9 dataplane scenarios). +- **`clears the flag on a gateway that becomes the hub side after a hub change`** — + a hub change turns a former spoke client (flag `true`) into the new hub's server; + the flag must be cleared to `false` so the hub doesn't route the whole slice, and + set on the gateway that becomes the new spoke client (direct test of the + both-sides reconcile). +- **`marks a no-network slice TopologyConverged=True with NoGatewaysRequired`** — a + NONET slice still gets a `TopologyConverged` condition (guards the early-return + path that previously skipped the status write). --- @@ -245,11 +254,20 @@ dataplane reads: File: `pkg/sidecar/sidecarpb/route_split_test.go`. -**`TestMoreSpecificHalves`** — the route split, 3 cases: +**`TestMoreSpecificHalves`** — the route split, 5 cases: - `10.11.0.0/16` → `10.11.0.0/17` + `10.11.128.0/17` - `10.11.0.0/20` → `10.11.0.0/21` + `10.11.8.0/21` - `10.11.32.3/32` → unchanged (a host route is not split) +- `10.11.0.0/31` → `10.11.0.0/32` + `10.11.0.1/32` (smallest splittable v4) +- `fd00::/48` → `fd00::/49` + `fd00:0:0:8000::/49` (IPv6 splits too) + +**`TestStaleTunnelRouteKeys`** — the teardown diff. On a topology/subnet change the +sidecar withdraws the tunnel routes it previously installed that are no longer +desired (e.g. the entire-slice `/17`s when a slice flips HubAndSpoke→FullMesh), +so a stale relay route isn't left behind. Verifies the previous-vs-desired set +difference: full flip (both old routes stale), no-change (nothing stale), and +partial overlap (only the non-desired key stale). **Why split at all:** NSM continuously re-asserts a route for the whole slice subnet via `nsm0` using the *same* prefix the tunnel wants. A `RouteReplace` on @@ -273,10 +291,10 @@ worker derives each gateway's tunnel state from its pod statuses and reports it onto the hub's `WorkerSliceGateway` status — the write side of the fields checked live in [Section 5 E12](#5-dataplane-end-to-end-runbook-openvpn). -- **`TestDeriveGatewayConnectionState`** — 6 cases: no pod status → `Pending`; +- **`TestDeriveGatewayConnectionState`** — 7 cases: no pod status → `Pending`; all pods up → `Connected`; at least one up → `Connected` (HA pair); all down → `NotConnected`; unknown/empty pod states are not counted as up; nil pod entries - are ignored. + are ignored; all-nil pod entries (nothing reported) → `Pending`. - **`TestReconcileGatewayConnectionStatus`** — 2 subtests: writes `Connected` when a tunnel is up; **no write when the state is unchanged** (idempotent — avoids status churn). From 05a89e0060a71407412f24e446ff8dce747e889c Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Fri, 28 Aug 2026 23:37:12 +0530 Subject: [PATCH 20/21] self-heal partial gateway pair on AlreadyExists; keep NONET test mocks consistent with status reconcile; add partial-pair envtest Signed-off-by: Shreesha001 --- .../sliceconfig_hubandspoke_test.go | 56 +++++++++++++++++++ docs/hub-and-spoke-testing.md | 5 +- service/slice_config_service_test.go | 7 ++- service/worker_slice_gateway_service.go | 54 ++++++++++-------- 4 files changed, 98 insertions(+), 24 deletions(-) diff --git a/controllers/controller/sliceconfig_hubandspoke_test.go b/controllers/controller/sliceconfig_hubandspoke_test.go index 48b66015..f1a69637 100644 --- a/controllers/controller/sliceconfig_hubandspoke_test.go +++ b/controllers/controller/sliceconfig_hubandspoke_test.go @@ -330,6 +330,62 @@ var _ = Describe("SliceConfig HubAndSpoke topology (partial mesh)", Ordered, fun } }) + It("re-creates a missing client gateway of a partial pair (self-heal)", func() { + // Simulates an interrupted pair creation: the server gateway exists but the + // client is gone. The reconcile must create the missing client instead of + // getting stuck on an AlreadyExists error when it re-touches the server. + const psName = "partial-slice" + key := types.NamespacedName{Name: psName, Namespace: nsName} + serverGw := psName + "-worker-1-worker-2" // hub server side + clientGw := psName + "-worker-2-worker-1" // spoke client side + + slice := &v1alpha1.SliceConfig{ + ObjectMeta: metav1.ObjectMeta{Name: psName, Namespace: nsName}, + Spec: v1alpha1.SliceConfigSpec{ + Clusters: []string{"worker-1", "worker-2", "worker-3"}, + MaxClusters: 4, + SliceSubnet: "10.14.0.0/16", + SliceGatewayProvider: &v1alpha1.WorkerSliceGatewayProvider{ + SliceGatewayType: "OpenVPN", + SliceCaType: "Local", + }, + SliceIpamType: "Local", + SliceType: "Application", + Topology: &v1alpha1.TopologySpec{Mode: v1alpha1.TopologyModeHubAndSpoke, Hubs: []string{"worker-1"}}, + QosProfileDetails: &v1alpha1.QOSProfile{ + BandwidthCeilingKbps: 5120, + DscpClass: "AF11", + }, + }, + } + Expect(k8sClient.Create(ctx, slice)).Should(Succeed()) + Eventually(func() bool { return gatewayExists(serverGw) && gatewayExists(clientGw) }, timeout, interval).Should(BeTrue()) + + // delete only the client -> partial pair (server present, client missing) + cgw := getGateway(clientGw) + Expect(k8sClient.Delete(ctx, &cgw)).Should(Succeed()) + + // nudge a reconcile and assert the client is re-created (the server, which + // still exists, must not block this with AlreadyExists) + latest := &v1alpha1.SliceConfig{} + Expect(k8sClient.Get(ctx, key, latest)).Should(Succeed()) + if latest.Labels == nil { + latest.Labels = map[string]string{} + } + latest.Labels["reconcile-nudge"] = "1" + Expect(k8sClient.Update(ctx, latest)).Should(Succeed()) + + Eventually(func() bool { return gatewayExists(clientGw) }, timeout, interval).Should(BeTrue()) + Expect(gatewayExists(serverGw)).To(BeTrue()) + + // cleanup (best-effort) + if k8sClient.Get(ctx, key, latest) == nil { + latest.Spec.Clusters = []string{} + _ = k8sClient.Update(ctx, latest) + _ = k8sClient.Delete(ctx, latest) + } + }) + It("marks a no-network slice TopologyConverged=True with NoGatewaysRequired", func() { // A no-network (NONET) slice has no gateway links, so it must still report a // TopologyConverged condition (True / NoGatewaysRequired). This guards the diff --git a/docs/hub-and-spoke-testing.md b/docs/hub-and-spoke-testing.md index 912063a6..d74bd6a0 100644 --- a/docs/hub-and-spoke-testing.md +++ b/docs/hub-and-spoke-testing.md @@ -206,7 +206,7 @@ single `TopologyConverged` condition on the SliceConfig. 5 cases: ## 4. Controller reconciler tests (envtest) File: `controllers/controller/sliceconfig_hubandspoke_test.go` (Ginkgo, real API -server via envtest). 5 specs: +server via envtest). 6 specs: - **`builds only hub<->spoke gateways and skips spoke<->spoke`** — applying a HubAndSpoke SliceConfig creates the 4 WorkerSliceGateway objects (2 pairs) and @@ -225,6 +225,9 @@ server via envtest). 5 specs: - **`marks a no-network slice TopologyConverged=True with NoGatewaysRequired`** — a NONET slice still gets a `TopologyConverged` condition (guards the early-return path that previously skipped the status write). +- **`re-creates a missing client gateway of a partial pair (self-heal)`** — with the + server gateway present but the client deleted, the reconcile re-creates the client + instead of getting stuck on an AlreadyExists error re-touching the server. --- diff --git a/service/slice_config_service_test.go b/service/slice_config_service_test.go index bc1aaddc..a7ac59e4 100644 --- a/service/slice_config_service_test.go +++ b/service/slice_config_service_test.go @@ -157,7 +157,7 @@ func SliceConfigReconciliationCompleteHappyCase(t *testing.T) { } func SliceConfigReconciliationNoNetCompleteHappyCase(t *testing.T) { - _, workerSliceConfigMock, _, _, _, clientMock, sliceConfig, ctx, sliceConfigService, requestObj, mMock := setupSliceConfigTest("slice_config", "namespace") + workerSliceGatewayMock, workerSliceConfigMock, _, _, _, clientMock, sliceConfig, ctx, sliceConfigService, requestObj, mMock := setupSliceConfigTest("slice_config", "namespace") mMock.On("WithProject", mock.AnythingOfType("string")).Return(&metrics.MetricRecorder{}).Once() clientMock.On("Get", ctx, requestObj.NamespacedName, sliceConfig).Return(nil).Run(func(args mock.Arguments) { arg := args.Get(2).(*controllerv1alpha1.SliceConfig) @@ -177,6 +177,10 @@ func SliceConfigReconciliationNoNetCompleteHappyCase(t *testing.T) { clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(nil) workerSliceConfigMock.On("CreateMinimalWorkerSliceConfigForNoNetworkSlice", ctx, mock.Anything, requestObj.Namespace, mock.Anything, mock.Anything).Return(nil).Once() + // A no-network slice reconciles its TopologyConverged status (zero gateways -> + // True/NoGatewaysRequired), which lists gateways and writes the slice status. + workerSliceGatewayMock.On("ListWorkerSliceGateways", ctx, mock.Anything, requestObj.Namespace).Return([]workerv1alpha1.WorkerSliceGateway{}, nil).Once() + clientMock.On("Status").Return(&fakeStatusWriter{}) result, err := sliceConfigService.ReconcileSliceConfig(ctx, requestObj) expectedResult := ctrl.Result{} @@ -185,6 +189,7 @@ func SliceConfigReconciliationNoNetCompleteHappyCase(t *testing.T) { require.False(t, result.Requeue) clientMock.AssertExpectations(t) workerSliceConfigMock.AssertExpectations(t) + workerSliceGatewayMock.AssertExpectations(t) mMock.AssertExpectations(t) } diff --git a/service/worker_slice_gateway_service.go b/service/worker_slice_gateway_service.go index f950c6d8..30a0cc5a 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -29,6 +29,7 @@ import ( "github.com/kubeslice/kubeslice-controller/metrics" corev1 "k8s.io/api/core/v1" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1" @@ -549,7 +550,11 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont gatewayAddresses.ServerSubnet, gatewayAddresses.ServerVpnAddress, clientGatewayName, gatewayAddresses.ClientSubnet, gatewayAddresses.ClientVpnAddress, serverGatewayName) err = util.CreateResource(ctx, serverGatewayObject) - if err != nil { + // Ignore AlreadyExists: the server gateway can already be present when a prior + // pair creation was interrupted after the server but before the client, or on a + // parallel reconcile. Falling through lets the missing client be created so the + // pair self-heals instead of getting stuck on an AlreadyExists error. + if err != nil && !k8sErrors.IsAlreadyExists(err) { //Register an event for worker slice gateway creation failure util.RecordEvent(ctx, eventRecorder, serverGatewayObject, nil, events.EventWorkerSliceGatewayCreationFailed) s.mf.RecordCounterMetric(metrics.KubeSliceEventsCounter, @@ -562,16 +567,18 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont ) return err } - //Register an event for worker slice gateway creation success - util.RecordEvent(ctx, eventRecorder, serverGatewayObject, nil, events.EventWorkerSliceGatewayCreated) - s.mf.RecordCounterMetric(metrics.KubeSliceEventsCounter, - map[string]string{ - "action": "created", - "event": string(events.EventWorkerSliceGatewayCreated), - "object_name": serverGatewayObject.Name, - "object_kind": metricKindWorkerSliceGateway, - }, - ) + if err == nil { + //Register an event for worker slice gateway creation success + util.RecordEvent(ctx, eventRecorder, serverGatewayObject, nil, events.EventWorkerSliceGatewayCreated) + s.mf.RecordCounterMetric(metrics.KubeSliceEventsCounter, + map[string]string{ + "action": "created", + "event": string(events.EventWorkerSliceGatewayCreated), + "object_name": serverGatewayObject.Name, + "object_kind": metricKindWorkerSliceGateway, + }, + ) + } clientGatewayObject := s.buildMinimumGateway(destinationCluster, sourceCluster, sliceName, namespace, clientGateway, gatewayConnType, gatewayProtocol, label, gatewayNumber, gatewayAddresses.ClientSubnet, gatewayAddresses.ClientVpnAddress, @@ -581,7 +588,8 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont // relayed through the hub. clientGatewayObject.Spec.RouteEntireSliceSubnet = routeEntireSliceSubnet err = util.CreateResource(ctx, clientGatewayObject) - if err != nil { + // Ignore AlreadyExists for the same idempotency/self-heal reason as the server. + if err != nil && !k8sErrors.IsAlreadyExists(err) { //Register an event for worker slice gateway creation failure util.RecordEvent(ctx, eventRecorder, clientGatewayObject, nil, events.EventWorkerSliceGatewayCreationFailed) s.mf.RecordCounterMetric(metrics.KubeSliceEventsCounter, @@ -594,16 +602,18 @@ func (s *WorkerSliceGatewayService) createMinimumGateWayPairIfNotExists(ctx cont ) return err } - //Register an event for worker slice gateway creation success - util.RecordEvent(ctx, eventRecorder, clientGatewayObject, nil, events.EventWorkerSliceGatewayCreated) - s.mf.RecordCounterMetric(metrics.KubeSliceEventsCounter, - map[string]string{ - "action": "created", - "event": string(events.EventWorkerSliceGatewayCreated), - "object_name": clientGatewayObject.Name, - "object_kind": metricKindWorkerSliceGateway, - }, - ) + if err == nil { + //Register an event for worker slice gateway creation success + util.RecordEvent(ctx, eventRecorder, clientGatewayObject, nil, events.EventWorkerSliceGatewayCreated) + s.mf.RecordCounterMetric(metrics.KubeSliceEventsCounter, + map[string]string{ + "action": "created", + "event": string(events.EventWorkerSliceGatewayCreated), + "object_name": clientGatewayObject.Name, + "object_kind": metricKindWorkerSliceGateway, + }, + ) + } err = s.GenerateCerts(ctx, sliceName, namespace, gatewayProtocol, serverGatewayObject, clientGatewayObject, gatewayAddresses) if err != nil { From 3a54ff1a08a379928e045a66fdc85daf0bc37199 Mon Sep 17 00:00:00 2001 From: Shreesha001 Date: Fri, 28 Aug 2026 23:56:08 +0530 Subject: [PATCH 21/21] requeue instead of silently skipping gateway creation when a member cluster CR is not found yet Signed-off-by: Shreesha001 --- service/worker_slice_gateway_service.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/service/worker_slice_gateway_service.go b/service/worker_slice_gateway_service.go index 30a0cc5a..3c901f12 100644 --- a/service/worker_slice_gateway_service.go +++ b/service/worker_slice_gateway_service.go @@ -455,9 +455,15 @@ func (s *WorkerSliceGatewayService) createMinimumGatewaysIfNotExists(ctx context } cluster := controllerv1alpha1.Cluster{} found, err := util.GetResourceIfExist(ctx, client.ObjectKey{Name: clusterName, Namespace: namespace}, &cluster) - if !found || err != nil { + if err != nil { return ctrl.Result{}, err } + if !found { + // A slice member's Cluster CR isn't present yet (e.g. registration + // lag). Return an error so the reconcile retries instead of silently + // skipping gateway creation for this and the remaining edges. + return ctrl.Result{}, fmt.Errorf("cluster %q not found while creating gateways for slice %q", clusterName, sliceName) + } clusterMapping[clusterName] = &cluster } }