From f8c34844edaa639d9ca5f7fc2b61b36fa40d1c3e Mon Sep 17 00:00:00 2001 From: michaelawyu Date: Wed, 26 Aug 2026 01:13:36 +0800 Subject: [PATCH 1/4] Updated the logic Signed-off-by: michaelawyu --- apis/cluster/v1beta1/zz_generated.deepcopy.go | 2 +- .../v1alpha1/zz_generated.deepcopy.go | 2 +- .../v1alpha1/zz_generated.deepcopy.go | 2 +- .../v1beta1/zz_generated.deepcopy.go | 2 +- .../clusterprofile/controller.go | 43 +++++++++---------- .../controller_integration_test.go | 4 ++ .../clusterprofile/controller_test.go | 7 ++- test/apis/v1alpha1/zz_generated.deepcopy.go | 2 +- 8 files changed, 33 insertions(+), 31 deletions(-) diff --git a/apis/cluster/v1beta1/zz_generated.deepcopy.go b/apis/cluster/v1beta1/zz_generated.deepcopy.go index a51641c3f..cec52aa39 100644 --- a/apis/cluster/v1beta1/zz_generated.deepcopy.go +++ b/apis/cluster/v1beta1/zz_generated.deepcopy.go @@ -21,7 +21,7 @@ limitations under the License. package v1beta1 import ( - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) diff --git a/apis/kubefleet.dev/placement/v1alpha1/zz_generated.deepcopy.go b/apis/kubefleet.dev/placement/v1alpha1/zz_generated.deepcopy.go index 429b4e7fa..19f036041 100644 --- a/apis/kubefleet.dev/placement/v1alpha1/zz_generated.deepcopy.go +++ b/apis/kubefleet.dev/placement/v1alpha1/zz_generated.deepcopy.go @@ -21,7 +21,7 @@ limitations under the License. package v1alpha1 import ( - "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" ) diff --git a/apis/placement/v1alpha1/zz_generated.deepcopy.go b/apis/placement/v1alpha1/zz_generated.deepcopy.go index 6d1656d18..df9f5e6d7 100644 --- a/apis/placement/v1alpha1/zz_generated.deepcopy.go +++ b/apis/placement/v1alpha1/zz_generated.deepcopy.go @@ -22,7 +22,7 @@ package v1alpha1 import ( "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" - "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" ) diff --git a/apis/placement/v1beta1/zz_generated.deepcopy.go b/apis/placement/v1beta1/zz_generated.deepcopy.go index 73d66c8fa..b9ff2e710 100644 --- a/apis/placement/v1beta1/zz_generated.deepcopy.go +++ b/apis/placement/v1beta1/zz_generated.deepcopy.go @@ -21,7 +21,7 @@ limitations under the License. package v1beta1 import ( - "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" ) diff --git a/pkg/controllers/clusterinventory/clusterprofile/controller.go b/pkg/controllers/clusterinventory/clusterprofile/controller.go index 94517e40d..b464d567c 100644 --- a/pkg/controllers/clusterinventory/clusterprofile/controller.go +++ b/pkg/controllers/clusterinventory/clusterprofile/controller.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + clientcmdv1 "k8s.io/client-go/tools/clientcmd/api/v1" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" clusterinventory "sigs.k8s.io/cluster-inventory-api/apis/v1alpha1" @@ -180,8 +181,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, nil } -// fillInClusterStatus fills in the ClusterProfile status fields from the MemberCluster status. -// Currently, it only fills in the Kubernetes version field. +// fillInClusterStatus fills in the ClusterProfile status fields from the MemberCluster status, +// including the Kubernetes version and, when available, the cluster access provider. func (r *Reconciler) fillInClusterStatus(mc *clusterv1beta1.MemberCluster, cp *clusterinventory.ClusterProfile) { clusterPropertyCondition := meta.FindStatusCondition(mc.Status.Conditions, string(clusterv1beta1.ConditionTypeClusterPropertyCollectionSucceeded)) if !condition.IsConditionStatusTrue(clusterPropertyCondition, mc.Generation) { @@ -196,27 +197,25 @@ func (r *Reconciler) fillInClusterStatus(mc *clusterv1beta1.MemberCluster, cp *c Kubernetes: k8sversion.Value, } } - // Add the class access provider, we only have one so far - cp.Status.AccessProviders = []clusterinventory.AccessProvider{ - { - Name: controller.ClusterManagerName, - }, - } - // TODO throw and unexpected error if clusterEntryPoint is not found - // We don't have a way to get it yet - clusterEntry, exists := mc.Status.Properties[propertyprovider.ClusterEntryPointProperty] - if exists { - klog.V(3).InfoS("Get Kubernetes cluster entry point from member cluster status", "clusterEntryPoint", clusterEntry.Value, "clusterProfile", klog.KObj(cp)) - cp.Status.AccessProviders[0].Cluster.Server = clusterEntry.Value - } - // Get the CA Data - certificateAuthorityData, exists := mc.Status.Properties[propertyprovider.ClusterCertificateAuthorityProperty] - if exists { - klog.V(3).InfoS("Get Kubernetes cluster certificate authority data from member cluster status", "clusterProfile", klog.KObj(cp)) - cp.Status.AccessProviders[0].Cluster.CertificateAuthorityData = []byte(certificateAuthorityData.Value) + + // Add cluster access provider, if and only if a cluster entry point and the CA data exist as part of the + // cluster properties. + clusterEntrypoint, entryPtExists := mc.Status.Properties[propertyprovider.ClusterEntryPointProperty] + caData, caDataExists := mc.Status.Properties[propertyprovider.ClusterCertificateAuthorityProperty] + if entryPtExists && caDataExists { + cp.Status.AccessProviders = []clusterinventory.AccessProvider{ + { + Name: controller.ClusterManagerName, + Cluster: clientcmdv1.Cluster{ + Server: clusterEntrypoint.Value, + CertificateAuthorityData: []byte(caData.Value), + }, + }, + } } else { - // throw an alert - _ = controller.NewUnexpectedBehaviorError(fmt.Errorf("cluster certificate authority data not found in member cluster %s status", mc.Name)) + klog.V(2).InfoS("Cluster entry point and/or CA data is missing; skip adding cluster access provider to cluster profile status", + "memberCluster", klog.KObj(mc), "clusterProfile", klog.KObj(cp), + "clusterEntryPointExists", entryPtExists, "caDataExists", caDataExists) } } diff --git a/pkg/controllers/clusterinventory/clusterprofile/controller_integration_test.go b/pkg/controllers/clusterinventory/clusterprofile/controller_integration_test.go index 49c13c571..b855c6165 100644 --- a/pkg/controllers/clusterinventory/clusterprofile/controller_integration_test.go +++ b/pkg/controllers/clusterinventory/clusterprofile/controller_integration_test.go @@ -171,6 +171,10 @@ var _ = Describe("Test ClusterProfile Controller", func() { }, } mc.Status.Properties = map[clusterv1beta1.PropertyName]clusterv1beta1.PropertyValue{ + propertyprovider.ClusterEntryPointProperty: { + Value: "https://dummy-cluster-endpoint", + ObservationTime: metav1.Time{Time: time.Now()}, + }, propertyprovider.ClusterCertificateAuthorityProperty: { Value: "dummy-ca-data", ObservationTime: metav1.Time{Time: time.Now()}, diff --git a/pkg/controllers/clusterinventory/clusterprofile/controller_test.go b/pkg/controllers/clusterinventory/clusterprofile/controller_test.go index b51ca2a2a..bd5d3c572 100644 --- a/pkg/controllers/clusterinventory/clusterprofile/controller_test.go +++ b/pkg/controllers/clusterinventory/clusterprofile/controller_test.go @@ -84,7 +84,7 @@ func TestFillInClusterStatus(t *testing.T) { }, clusterProfile: &clusterinventory.ClusterProfile{}, expectVersion: false, - expectAccessProvider: true, + expectAccessProvider: false, }, { name: "Cluster property collection succeeded with k8s version only", @@ -111,7 +111,7 @@ func TestFillInClusterStatus(t *testing.T) { clusterProfile: &clusterinventory.ClusterProfile{}, expectVersion: true, expectedK8sVersion: "v1.28.0", - expectAccessProvider: true, + expectAccessProvider: false, }, { name: "Cluster property collection succeeded with all properties", @@ -176,8 +176,7 @@ func TestFillInClusterStatus(t *testing.T) { clusterProfile: &clusterinventory.ClusterProfile{}, expectVersion: true, expectedK8sVersion: "v1.27.5", - expectAccessProvider: true, - expectedServer: "https://api.partial-cluster.example.com:6443", + expectAccessProvider: false, }, } diff --git a/test/apis/v1alpha1/zz_generated.deepcopy.go b/test/apis/v1alpha1/zz_generated.deepcopy.go index 081bec913..143bdee7b 100644 --- a/test/apis/v1alpha1/zz_generated.deepcopy.go +++ b/test/apis/v1alpha1/zz_generated.deepcopy.go @@ -21,7 +21,7 @@ limitations under the License. package v1alpha1 import ( - "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) From e829e1dd786592559a252d4c0c71261741f0292f Mon Sep 17 00:00:00 2001 From: michaelawyu Date: Wed, 26 Aug 2026 09:24:12 +0800 Subject: [PATCH 2/4] Minor fixes Signed-off-by: michaelawyu --- .../clusterprofile/controller.go | 5 +- .../clusterprofile/controller_test.go | 73 +++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/pkg/controllers/clusterinventory/clusterprofile/controller.go b/pkg/controllers/clusterinventory/clusterprofile/controller.go index b464d567c..4e5346a4f 100644 --- a/pkg/controllers/clusterinventory/clusterprofile/controller.go +++ b/pkg/controllers/clusterinventory/clusterprofile/controller.go @@ -202,7 +202,7 @@ func (r *Reconciler) fillInClusterStatus(mc *clusterv1beta1.MemberCluster, cp *c // cluster properties. clusterEntrypoint, entryPtExists := mc.Status.Properties[propertyprovider.ClusterEntryPointProperty] caData, caDataExists := mc.Status.Properties[propertyprovider.ClusterCertificateAuthorityProperty] - if entryPtExists && caDataExists { + if entryPtExists && caDataExists && len(clusterEntrypoint.Value) > 0 && len(caData.Value) > 0 { cp.Status.AccessProviders = []clusterinventory.AccessProvider{ { Name: controller.ClusterManagerName, @@ -213,7 +213,8 @@ func (r *Reconciler) fillInClusterStatus(mc *clusterv1beta1.MemberCluster, cp *c }, } } else { - klog.V(2).InfoS("Cluster entry point and/or CA data is missing; skip adding cluster access provider to cluster profile status", + cp.Status.AccessProviders = nil + klog.V(2).InfoS("Cluster entry point and/or CA data is missing; reset cluster access provider to cluster profile status", "memberCluster", klog.KObj(mc), "clusterProfile", klog.KObj(cp), "clusterEntryPointExists", entryPtExists, "caDataExists", caDataExists) } diff --git a/pkg/controllers/clusterinventory/clusterprofile/controller_test.go b/pkg/controllers/clusterinventory/clusterprofile/controller_test.go index bd5d3c572..fed86ef32 100644 --- a/pkg/controllers/clusterinventory/clusterprofile/controller_test.go +++ b/pkg/controllers/clusterinventory/clusterprofile/controller_test.go @@ -23,6 +23,7 @@ import ( "github.com/google/go-cmp/cmp" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clientcmdv1 "k8s.io/client-go/tools/clientcmd/api/v1" clusterinventory "sigs.k8s.io/cluster-inventory-api/apis/v1alpha1" clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" @@ -178,6 +179,78 @@ func TestFillInClusterStatus(t *testing.T) { expectedK8sVersion: "v1.27.5", expectAccessProvider: false, }, + { + name: "Cluster property collection succeeded but access provider properties are empty", + memberCluster: &clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Generation: 1, + }, + Status: clusterv1beta1.MemberClusterStatus{ + Conditions: []metav1.Condition{ + { + Type: string(clusterv1beta1.ConditionTypeClusterPropertyCollectionSucceeded), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + }, + Properties: map[clusterv1beta1.PropertyName]clusterv1beta1.PropertyValue{ + propertyprovider.K8sVersionProperty: { + Value: "v1.30.0", + }, + propertyprovider.ClusterEntryPointProperty: { + Value: "", + }, + propertyprovider.ClusterCertificateAuthorityProperty: { + Value: "", + }, + }, + }, + }, + clusterProfile: &clusterinventory.ClusterProfile{}, + expectVersion: true, + expectedK8sVersion: "v1.30.0", + expectAccessProvider: false, + }, + { + name: "Access provider properties missing resets a previously populated access provider", + memberCluster: &clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Generation: 1, + }, + Status: clusterv1beta1.MemberClusterStatus{ + Conditions: []metav1.Condition{ + { + Type: string(clusterv1beta1.ConditionTypeClusterPropertyCollectionSucceeded), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + }, + Properties: map[clusterv1beta1.PropertyName]clusterv1beta1.PropertyValue{ + propertyprovider.K8sVersionProperty: { + Value: "v1.31.0", + }, + }, + }, + }, + clusterProfile: &clusterinventory.ClusterProfile{ + Status: clusterinventory.ClusterProfileStatus{ + AccessProviders: []clusterinventory.AccessProvider{ + { + Name: controller.ClusterManagerName, + Cluster: clientcmdv1.Cluster{ + Server: "https://api.stale-cluster.example.com:6443", + CertificateAuthorityData: []byte("c3RhbGUtY2EtZGF0YQ=="), + }, + }, + }, + }, + }, + expectVersion: true, + expectedK8sVersion: "v1.31.0", + expectAccessProvider: false, + }, } for _, tt := range tests { From 24bb2705c7877044160becbb929a7335d1f2f66e Mon Sep 17 00:00:00 2001 From: michaelawyu Date: Wed, 26 Aug 2026 09:33:55 +0800 Subject: [PATCH 3/4] Minor fixes Signed-off-by: michaelawyu --- pkg/controllers/clusterinventory/clusterprofile/controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controllers/clusterinventory/clusterprofile/controller.go b/pkg/controllers/clusterinventory/clusterprofile/controller.go index 4e5346a4f..66c23f342 100644 --- a/pkg/controllers/clusterinventory/clusterprofile/controller.go +++ b/pkg/controllers/clusterinventory/clusterprofile/controller.go @@ -214,7 +214,7 @@ func (r *Reconciler) fillInClusterStatus(mc *clusterv1beta1.MemberCluster, cp *c } } else { cp.Status.AccessProviders = nil - klog.V(2).InfoS("Cluster entry point and/or CA data is missing; reset cluster access provider to cluster profile status", + klog.V(2).InfoS("Cluster entry point and/or CA data is missing or empty; reset cluster access provider to cluster profile status", "memberCluster", klog.KObj(mc), "clusterProfile", klog.KObj(cp), "clusterEntryPointExists", entryPtExists, "caDataExists", caDataExists) } From 9b9f585176e94bc7804f6b65e35096c88134d7a6 Mon Sep 17 00:00:00 2001 From: michaelawyu Date: Wed, 26 Aug 2026 14:53:07 +0800 Subject: [PATCH 4/4] Minor fixes Signed-off-by: michaelawyu --- test/e2e/join_and_leave_test.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/test/e2e/join_and_leave_test.go b/test/e2e/join_and_leave_test.go index ce715bf2d..4cfc8260b 100644 --- a/test/e2e/join_and_leave_test.go +++ b/test/e2e/join_and_leave_test.go @@ -35,7 +35,6 @@ import ( clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" "github.com/kubefleet-dev/kubefleet/pkg/utils" - "github.com/kubefleet-dev/kubefleet/pkg/utils/controller" ) const ( @@ -462,14 +461,10 @@ var _ = Describe("Test member cluster join and leave with clusterProfile", Label if cp.Status.Version.Kubernetes == "" { return fmt.Errorf("cluster profile %s Kubernetes version should not be empty", cp.Name) } - if len(cp.Status.AccessProviders) != 1 { - return fmt.Errorf("cluster profile %s has no access providers %+v", cp.Name, cp.Status.AccessProviders) - } - if cp.Status.AccessProviders[0].Name != controller.ClusterManagerName { - return fmt.Errorf("cluster profile %s access provider name %s doesn't match expected %s", cp.Name, cp.Status.AccessProviders[0].Name, controller.ClusterManagerName) - } - if len(cp.Status.AccessProviders[0].Cluster.CertificateAuthorityData) == 0 { - return fmt.Errorf("cluster profile %s access provider certificate authority data should not be empty", allMemberClusterNames[idx]) + if len(cp.Status.AccessProviders) != 0 { + // Note: at this moment the Azure property provider does not expose cluster FQDNs, and + // as a result no access provider will be populated. + return fmt.Errorf("cluster profile %s has access providers %+v", cp.Name, cp.Status.AccessProviders) } } return nil