Fix nil pointer panic in upgrade

Signed-off-by: Jian Qiu <[email protected]>

pkg/cmd/upgrade/clustermanager/exec.go

@@ -52,10 +52,6 @@ func (o *Options) complete(cmd *cobra.Command, args []string) (err error) {
 		Hub: clusteradminit.Hub{
 			Registry: o.registry,
 		},
-		// reconstruct values from the cluster manager CR.
-		RegistrationFeatures: cm.Spec.RegistrationConfiguration.FeatureGates,
-		WorkFeatures:         cm.Spec.WorkConfiguration.FeatureGates,
-		AddonFeatures:        cm.Spec.AddOnManagerConfiguration.FeatureGates,
 		BundleVersion: clusteradminit.BundleVersion{
 			RegistrationImageVersion: versionBundle.Registration,
 			PlacementImageVersion:    versionBundle.Placement,
@@ -64,8 +60,18 @@ func (o *Options) complete(cmd *cobra.Command, args []string) (err error) {
 		},
 	}
 
-	if len(cm.Spec.RegistrationConfiguration.AutoApproveUsers) > 0 {
-		o.values.AutoApprove = true
+	// reconstruct values from the cluster manager CR.
+	if cm.Spec.RegistrationConfiguration != nil {
+		o.values.RegistrationFeatures = cm.Spec.RegistrationConfiguration.FeatureGates
+		if len(cm.Spec.RegistrationConfiguration.AutoApproveUsers) > 0 {
+			o.values.AutoApprove = true
+		}
+	}
+	if cm.Spec.WorkConfiguration != nil {
+		o.values.WorkFeatures = cm.Spec.WorkConfiguration.FeatureGates
+	}
+	if cm.Spec.AddOnManagerConfiguration != nil {
+		o.values.AddonFeatures = cm.Spec.AddOnManagerConfiguration.FeatureGates
 	}
 
 	return nil

pkg/cmd/upgrade/klusterlet/exec.go

@@ -33,7 +33,10 @@ func (o *Options) complete(cmd *cobra.Command, args []string) (err error) {
 		return err
 	}
 
-	cfg, err := o.ClusteradmFlags.KubectlFactory.ToRESTConfig()
+	f := o.ClusteradmFlags.KubectlFactory
+	o.builder = f.NewBuilder()
+
+	cfg, err := f.ToRESTConfig()
 	if err != nil {
 		return err
 	}
@@ -56,10 +59,8 @@ func (o *Options) complete(cmd *cobra.Command, args []string) (err error) {
 
 	klog.V(1).InfoS("init options:", "dry-run", o.ClusteradmFlags.DryRun)
 	o.values = join.Values{
-		Registry:             o.registry,
-		ClusterName:          k.Spec.ClusterName,
-		RegistrationFeatures: k.Spec.RegistrationConfiguration.FeatureGates,
-		WorkFeatures:         k.Spec.WorkConfiguration.FeatureGates,
+		Registry:    o.registry,
+		ClusterName: k.Spec.ClusterName,
 		BundleVersion: join.BundleVersion{
 			RegistrationImageVersion: versionBundle.Registration,
 			PlacementImageVersion:    versionBundle.Placement,
@@ -68,6 +69,14 @@ func (o *Options) complete(cmd *cobra.Command, args []string) (err error) {
 		},
 	}
 
+	// reconstruct values from the klusterlet CR.
+	if k.Spec.RegistrationConfiguration != nil {
+		o.values.RegistrationFeatures = k.Spec.RegistrationConfiguration.FeatureGates
+	}
+	if k.Spec.WorkConfiguration != nil {
+		o.values.WorkFeatures = k.Spec.WorkConfiguration.FeatureGates
+	}
+
 	return nil
 }
 
@@ -104,8 +113,6 @@ func (o *Options) run() error {
 	}
 
 	files := []string{
-		"join/namespace_agent.yaml",
-		"join/namespace_addon.yaml",
 		"join/namespace.yaml",
 		"join/cluster_role.yaml",
 		"join/cluster_role_binding.yaml",
@@ -124,7 +131,7 @@ func (o *Options) run() error {
 	}
 
 	if !o.ClusteradmFlags.DryRun {
-		if err := wait.WaitUntilCRDReady(apiExtensionsClient, "clustermanagers.operator.open-cluster-management.io", o.wait); err != nil {
+		if err := wait.WaitUntilCRDReady(apiExtensionsClient, "klusterlets.operator.open-cluster-management.io", o.wait); err != nil {
 			return err
 		}
 	}

test/e2e/clusteradm/upgrade_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/onsi/ginkgo/v2"
 	"github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
 	"open-cluster-management.io/clusteradm/pkg/helpers/version"
 )
 
@@ -22,14 +23,14 @@ var _ = ginkgo.Describe("test clusteradm upgrade clustermanager and Klusterlets"
 
 	var err error
 
-	ginkgo.It("run cluster manager upgrade version latest ", func() {
+	ginkgo.It("run cluster manager and klusterlet upgrade version latest ", func() {
 		ginkgo.By("init hub with service account")
 		err = e2e.Clusteradm().Init(
 			"--context", e2e.Cluster().Hub().Context(),
 		)
 		gomega.Expect(err).NotTo(gomega.HaveOccurred(), "clusteradm init error")
 
-		ginkgo.By("Check the version of operator and agent")
+		ginkgo.By("Check the version of operator and controller")
 		gomega.Eventually(func() error {
 			operator, err := kubeClient.AppsV1().Deployments("open-cluster-management").Get(context.TODO(), "cluster-manager", metav1.GetOptions{})
 			if err != nil {
@@ -43,6 +44,39 @@ var _ = ginkgo.Describe("test clusteradm upgrade clustermanager and Klusterlets"
 			if err != nil {
 				return err
 			}
+			if registration.Spec.Template.Spec.Containers[0].Image != "quay.io/open-cluster-management/registration:v"+version.GetDefaultBundleVersion() {
+				return fmt.Errorf("version of the registration controller is not correct, get %s", operator.Spec.Template.Spec.Containers[0].Image)
+			}
+			return nil
+		}, 120*time.Second, 5*time.Second).Should(gomega.Succeed())
+
+		ginkgo.By("managedcluster1 join hub")
+		err = e2e.Clusteradm().Join(
+			"--context", e2e.Cluster().ManagedCluster1().Context(),
+			"--hub-token", e2e.CommandResult().Token(), "--hub-apiserver", e2e.CommandResult().Host(),
+			"--cluster-name", e2e.Cluster().ManagedCluster1().Name(),
+			"--wait",
+			"--force-internal-endpoint-lookup",
+		)
+		gomega.Expect(err).NotTo(gomega.HaveOccurred(), "managedcluster1 join error")
+
+		mcl1KubeClient, err := kubernetes.NewForConfig(e2e.Cluster().ManagedCluster1().KubeConfig())
+		gomega.Expect(err).NotTo(gomega.HaveOccurred())
+
+		ginkgo.By("Check the version of operator and agent")
+		gomega.Eventually(func() error {
+			operator, err := mcl1KubeClient.AppsV1().Deployments("open-cluster-management").Get(context.TODO(), "klusterlet", metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+			if operator.Spec.Template.Spec.Containers[0].Image != "quay.io/open-cluster-management/registration-operator:v"+version.GetDefaultBundleVersion() {
+				return fmt.Errorf("version of the operator is not correct, get %s", operator.Spec.Template.Spec.Containers[0].Image)
+			}
+			registration, err := mcl1KubeClient.AppsV1().Deployments("open-cluster-management-agent").Get(
+				context.TODO(), "klusterlet-registration-agent", metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
 			if registration.Spec.Template.Spec.Containers[0].Image != "quay.io/open-cluster-management/registration:v"+version.GetDefaultBundleVersion() {
 				return fmt.Errorf("version of the registration agent is not correct, get %s", operator.Spec.Template.Spec.Containers[0].Image)
 			}
@@ -72,8 +106,35 @@ var _ = ginkgo.Describe("test clusteradm upgrade clustermanager and Klusterlets"
 				return err
 			}
 			if registration.Spec.Template.Spec.Containers[0].Image != "quay.io/open-cluster-management/registration:latest" {
+				return fmt.Errorf("version of the controller is not correct, get %s", operator.Spec.Template.Spec.Containers[0].Image)
+			}
+			return nil
+		}, 120*time.Second, 5*time.Second).Should(gomega.Succeed())
+
+		err = e2e.Clusteradm().Upgrade(
+			"klusterlet",
+			"--bundle-version=latest",
+			"--context", e2e.Cluster().ManagedCluster1().Context(),
+		)
+
+		gomega.Expect(err).NotTo(gomega.HaveOccurred(), "klusterlet upgrade error")
+
+		gomega.Eventually(func() error {
+			operator, err := kubeClient.AppsV1().Deployments("open-cluster-management").Get(context.TODO(), "klusterlet", metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+			if operator.Spec.Template.Spec.Containers[0].Image != "quay.io/open-cluster-management/registration-operator:latest" {
 				return fmt.Errorf("version of the operator is not correct, get %s", operator.Spec.Template.Spec.Containers[0].Image)
 			}
+			registration, err := kubeClient.AppsV1().Deployments("open-cluster-management-agent").Get(
+				context.TODO(), "klusterlet-registration-agent", metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+			if registration.Spec.Template.Spec.Containers[0].Image != "quay.io/open-cluster-management/registration:latest" {
+				return fmt.Errorf("version of the agent is not correct, get %s", operator.Spec.Template.Spec.Containers[0].Image)
+			}
 			return nil
 		}, 120*time.Second, 5*time.Second).Should(gomega.Succeed())
 	})

test/e2e/util/e2econf.go

@@ -28,16 +28,26 @@ func NewTestE2eConfig(
 	hubctx string,
 	mcl1 string,
 	mcl1ctx string,
-) *TestE2eConfig {
+) (*TestE2eConfig, error) {
 
+	hubConfig, err := buildConfigFromFlags(hubctx, kubeconfigpath)
+	if err != nil {
+		return nil, err
+	}
+	mcl1Config, err := buildConfigFromFlags(mcl1ctx, kubeconfigpath)
+	if err != nil {
+		return nil, err
+	}
 	ctx := clusterValues{
 		hub: &clusterConfig{
-			name:    hub,
-			context: hubctx,
+			name:       hub,
+			context:    hubctx,
+			kubeConfig: hubConfig,
 		},
 		mcl1: &clusterConfig{
-			name:    mcl1,
-			context: mcl1ctx,
+			name:       mcl1,
+			context:    mcl1ctx,
+			kubeConfig: mcl1Config,
 		},
 	}
 
@@ -49,5 +59,5 @@ func NewTestE2eConfig(
 		values:         &cfgval,
 		clusteradm:     &clusteradm{},
 		Kubeconfigpath: kubeconfigpath,
-	}
+	}, nil
 }

test/e2e/util/helper.go

@@ -3,10 +3,8 @@ package util
 
 import (
 	"context"
-	"fmt"
 	"time"
 
-	apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -18,12 +16,7 @@ import (
 
 // WaitNamespaceDeleted receive a kubeconfigpath, a context name and a namespace name,
 // then poll until the specific namespace is fully deleted or an error occurs.
-func WaitNamespaceDeleted(kubeconfigpath string, ctx string, namespace string) error {
-	restcfg, err := buildConfigFromFlags(ctx, kubeconfigpath)
-	if err != nil {
-		return fmt.Errorf("error occurred while build rest config: %s", err)
-	}
-
+func WaitNamespaceDeleted(restcfg *rest.Config, namespace string) error {
 	clientset, err := kubernetes.NewForConfig(restcfg)
 	if err != nil {
 		return err
@@ -41,12 +34,7 @@ func WaitNamespaceDeleted(kubeconfigpath string, ctx string, namespace string) e
 	})
 }
 
-func DeleteClusterCSRs(kubeconfigpath string, ctx string) error {
-	restcfg, err := buildConfigFromFlags(ctx, kubeconfigpath)
-	if err != nil {
-		return fmt.Errorf("error occurred while build rest config: %s", err)
-	}
-
+func DeleteClusterCSRs(restcfg *rest.Config) error {
 	clientset, err := kubernetes.NewForConfig(restcfg)
 	if err != nil {
 		return err
@@ -57,12 +45,7 @@ func DeleteClusterCSRs(kubeconfigpath string, ctx string) error {
 	})
 }
 
-func DeleteClusterFinalizers(kubeconfigpath string, ctx string) error {
-	restcfg, err := buildConfigFromFlags(ctx, kubeconfigpath)
-	if err != nil {
-		return fmt.Errorf("error occurred while build rest config: %s", err)
-	}
-
+func DeleteClusterFinalizers(restcfg *rest.Config) error {
 	clientset, err := clusterclient.NewForConfig(restcfg)
 	if err != nil {
 		return err
@@ -82,29 +65,6 @@ func DeleteClusterFinalizers(kubeconfigpath string, ctx string) error {
 	return nil
 }
 
-func WaitCRDDeleted(kubeconfigpath string, ctx string, name string) error {
-	restcfg, err := buildConfigFromFlags(ctx, kubeconfigpath)
-	if err != nil {
-		return fmt.Errorf("error occurred while build rest config: %s", err)
-	}
-
-	client, err := apiextensionsclient.NewForConfig(restcfg)
-	if err != nil {
-		return err
-	}
-
-	return wait.PollUntilContextCancel(context.TODO(), 1*time.Second, true, func(ctx context.Context) (bool, error) {
-		_, err = client.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, name, metav1.GetOptions{})
-		if errors.IsNotFound(err) {
-			return true, nil
-		}
-		if err != nil {
-			return false, err
-		}
-		return false, nil
-	})
-}
-
 // buildConfigFromFlags build rest config for specified context in the kubeconfigfile.
 func buildConfigFromFlags(context, kubeconfigPath string) (*rest.Config, error) {
 	return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(

test/e2e/util/util.go

@@ -53,11 +53,14 @@ func initE2E() (*TestE2eConfig, error) {
 		os.Setenv("KUBECONFIG", filepath.Join(home, ".kube", "config"))
 	}
 
-	e2eConf := NewTestE2eConfig(
+	e2eConf, err := NewTestE2eConfig(
 		os.Getenv("KUBECONFIG"),
 		os.Getenv("HUB_NAME"), os.Getenv("HUB_CTX"),
 		os.Getenv("MANAGED_CLUSTER1_NAME"), os.Getenv("MANAGED_CLUSTER1_CTX"),
 	)
+	if err != nil {
+		return nil, err
+	}
 
 	// clearenv set the e2e environment from initial state to empty
 	clearenv := func() error {
@@ -71,13 +74,13 @@ func initE2E() (*TestE2eConfig, error) {
 		if err != nil {
 			return err
 		}
-		err = WaitNamespaceDeleted(e2eConf.Kubeconfigpath, e2eConf.Cluster().ManagedCluster1().Context(), config.ManagedClusterNamespace)
+		err = WaitNamespaceDeleted(e2eConf.Cluster().ManagedCluster1().KubeConfig(), config.ManagedClusterNamespace)
 		if err != nil {
 			return err
 		}
 
 		// delete cluster finalizers
-		err = DeleteClusterFinalizers(e2eConf.Kubeconfigpath, e2eConf.Cluster().Hub().Context())
+		err = DeleteClusterFinalizers(e2eConf.Cluster().Hub().KubeConfig())
 		if err != nil {
 			return err
 		}
@@ -89,11 +92,11 @@ func initE2E() (*TestE2eConfig, error) {
 			return err
 		}
 
-		err = DeleteClusterCSRs(e2eConf.Kubeconfigpath, e2eConf.Cluster().Hub().Context())
+		err = DeleteClusterCSRs(e2eConf.Cluster().Hub().KubeConfig())
 		if err != nil {
 			return err
 		}
-		err = WaitNamespaceDeleted(e2eConf.Kubeconfigpath, e2eConf.Cluster().Hub().Context(), config.HubClusterNamespace)
+		err = WaitNamespaceDeleted(e2eConf.Cluster().Hub().KubeConfig(), config.HubClusterNamespace)
 		if err != nil {
 			return err
 		}

test/e2e/util/values.go

@@ -1,9 +1,12 @@
 // Copyright Contributors to the Open Cluster Management project
 package util
 
+import "k8s.io/client-go/rest"
+
 type clusterConfig struct {
-	name    string
-	context string
+	name       string
+	context    string
+	kubeConfig *rest.Config
 }
 
 func (cc *clusterConfig) Name() string {
@@ -14,6 +17,10 @@ func (cc *clusterConfig) Context() string {
 	return cc.context
 }
 
+func (cc *clusterConfig) KubeConfig() *rest.Config {
+	return cc.kubeConfig
+}
+
 type clusterValues struct {
 	hub  *clusterConfig
 	mcl1 *clusterConfig