Skip to content

Commit

Permalink
address comments.
Browse files Browse the repository at this point in the history 
Signed-off-by: morvencao <lcao@redhat.com>
  • Loading branch information
@morvencao
morvencao committed Mar 25, 2024
1 parent c5f756d commit 8e56daf
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 52 deletions.
43 changes: 2 additions & 41 deletions pkg/operator/helpers/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -725,45 +725,6 @@ func GetHubKubeconfig(ctx context.Context,
}
}

// SyncWorkConfigSecret is used to sync the secret of work config from operator namespace to the cluster manager namespace.
// For both Default and Hosted mode, the original secret of work config should be in the operator namespace.
func SyncWorkConfigSecret(ctx context.Context,
operatorClient kubernetes.Interface,
clusterManagerNamespace string,
) error {
// get the namespace of the operator
operatorNamespace, err := getOperatorNamespace()
if err != nil {
return err
}

// get secret of work config
workDriverSecret, err := operatorClient.CoreV1().Secrets(operatorNamespace).Get(ctx, WorkDriverConfig, metav1.GetOptions{})
if err != nil {
return err
}

// copy the secret of work config to new secret in the cluster manager namespace
newSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: workDriverSecret.Name,
Namespace: clusterManagerNamespace,
},
Data: workDriverSecret.Data,
}
// create or update secret of work config in the cluster manager namespace
_, err = operatorClient.CoreV1().Secrets(clusterManagerNamespace).Create(ctx, newSecret, metav1.CreateOptions{})
if err != nil {
if errors.IsAlreadyExists(err) {
_, err = operatorClient.CoreV1().Secrets(clusterManagerNamespace).Update(ctx, newSecret, metav1.UpdateOptions{})
return err
}
return err
}

return nil
}

// RemoveWorkConfigSecret is used to remove the secret of work config from the cluster manager namespace.
func RemoveWorkConfigSecret(ctx context.Context,
operatorClient kubernetes.Interface,
Expand All @@ -783,8 +744,8 @@ func RemoveWorkConfigSecret(ctx context.Context,
return err
}

// getOperatorNamespace is used to get the namespace where the operator is running.
func getOperatorNamespace() (string, error) {
// GetOperatorNamespace is used to get the namespace where the operator is running.
func GetOperatorNamespace() (string, error) {
podNamespace, exists := os.LookupEnv("POD_NAMESPACE")
if exists {
return podNamespace, nil
Expand Down
77 changes: 66 additions & 11 deletions ...perators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ import (
)

const (
clusterManagerFinalizer = "operator.open-cluster-management.io/cluster-manager-cleanup"
clusterManagerApplied = "Applied"
clusterManagerProgressing = "Progressing"
clusterManagerFinalizer = "operator.open-cluster-management.io/cluster-manager-cleanup"
clusterManagerApplied = "Applied"
clusterManagerProgressing = "Progressing"
workDriverConfigSecretSynced = "WorkDriverConfigSecretSynced"

defaultWebhookPort = int32(9443)
clusterManagerReSyncTime = 5 * time.Second
Expand Down Expand Up @@ -216,14 +217,68 @@ func (n *clusterManagerController) sync(ctx context.Context, controllerContext f
}
managementClient := n.operatorKubeClient // We assume that operator is always running on the management cluster.

// If the work driver is not kube, we need to sync the work driver config secret
if workDriver != operatorapiv1.WorkDriverTypeKube {
if err := helpers.SyncWorkConfigSecret(ctx, n.operatorKubeClient, clusterManagerNamespace); err != nil {
return err
}
} else {
if err := helpers.RemoveWorkConfigSecret(ctx, n.operatorKubeClient, clusterManagerNamespace); err != nil {
return err
if config.MWReplicaSetEnabled {
// If the work driver is not kube, we need to sync the work driver config secret
if workDriver != operatorapiv1.WorkDriverTypeKube {
operatorNamespace, err := helpers.GetOperatorNamespace()
if err != nil {
return err
}
// check the secret containing work driver config explicitly because
// resourceapply.SyncSecret below won't return err if the source secret is not found
_, err = n.operatorKubeClient.CoreV1().Secrets(operatorNamespace).Get(ctx, helpers.WorkDriverConfig, metav1.GetOptions{})
if err != nil {
if errors.IsNotFound(err) {
klog.Errorf("Secret %s not found in namespace %s", helpers.WorkDriverConfig, operatorNamespace)
conditionChanged := meta.SetStatusCondition(&clusterManager.Status.Conditions, metav1.Condition{
Type: workDriverConfigSecretSynced,
Status: metav1.ConditionFalse,
Reason: "WorkDriverConfigSecretNotFound",
Message: "Work driver config secret is not found",
})
if conditionChanged {
clusterManager.Status.ObservedGeneration = clusterManager.Generation
_, err := n.patcher.PatchStatus(ctx, clusterManager, clusterManager.Status, originalClusterManager.Status)
if err != nil {
return err
}
}
}
return err
} else {
_, _, err = resourceapply.SyncSecret(ctx, n.operatorKubeClient.CoreV1(), n.recorder,
operatorNamespace, helpers.WorkDriverConfig, clusterManagerNamespace, helpers.WorkDriverConfig,
[]metav1.OwnerReference{})
if err != nil {
return err
}
conditionChanged := meta.SetStatusCondition(&clusterManager.Status.Conditions, metav1.Condition{
Type: workDriverConfigSecretSynced,
Status: metav1.ConditionTrue,
Reason: "WorkDriverConfigSecretSynced",
Message: "Work driver config secret is synced",
})
if conditionChanged {
clusterManager.Status.ObservedGeneration = clusterManager.Generation
_, err := n.patcher.PatchStatus(ctx, clusterManager, clusterManager.Status, originalClusterManager.Status)
if err != nil {
return err
}
}
}
} else {
// If the work driver is kube, we need to remove the work driver config secret if it exists
if err := helpers.RemoveWorkConfigSecret(ctx, n.operatorKubeClient, clusterManagerNamespace); err != nil {
return err
}
conditionChanged := meta.RemoveStatusCondition(&clusterManager.Status.Conditions, workDriverConfigSecretSynced)
if conditionChanged {
clusterManager.Status.ObservedGeneration = clusterManager.Generation
_, err := n.patcher.PatchStatus(ctx, clusterManager, clusterManager.Status, originalClusterManager.Status)
if err != nil {
return err
}
}
}
}

Expand Down
41 changes: 41 additions & 0 deletions test/integration/operator/clustermanager_hosted_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/util/cert"
Expand Down Expand Up @@ -502,6 +503,18 @@ var _ = ginkgo.Describe("ClusterManager Hosted Mode", func() {
return err
}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil())

gomega.Eventually(func() error {
actual, err := hostedOperatorClient.OperatorV1().ClusterManagers().Get(
context.Background(), clusterManagerName, metav1.GetOptions{})
if err != nil {
return err
}
if !meta.IsStatusConditionFalse(actual.Status.Conditions, "WorkDriverConfigSecretSynced") {
return fmt.Errorf("should get WorkDriverConfigSecretSynced condition false")
}
return nil
}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())

_, err := hostedKubeClient.CoreV1().Secrets("default").Create(context.TODO(), &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: helpers.WorkDriverConfig,
Expand All @@ -513,6 +526,18 @@ var _ = ginkgo.Describe("ClusterManager Hosted Mode", func() {
}, metav1.CreateOptions{})
gomega.Expect(err).NotTo(gomega.HaveOccurred())

gomega.Eventually(func() error {
actual, err := hostedOperatorClient.OperatorV1().ClusterManagers().Get(
context.Background(), clusterManagerName, metav1.GetOptions{})
if err != nil {
return err
}
if !meta.IsStatusConditionTrue(actual.Status.Conditions, "WorkDriverConfigSecretSynced") {
return fmt.Errorf("should get WorkDriverConfigSecretSynced condition true")
}
return nil
}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())

gomega.Eventually(func() error {
actual, err := hostedKubeClient.AppsV1().Deployments(hubNamespaceHosted).Get(context.Background(),
hubWorkControllerDeployment, metav1.GetOptions{})
Expand Down Expand Up @@ -565,6 +590,18 @@ var _ = ginkgo.Describe("ClusterManager Hosted Mode", func() {
return err
}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil())

gomega.Eventually(func() error {
actual, err := hostedOperatorClient.OperatorV1().ClusterManagers().Get(
context.Background(), clusterManagerName, metav1.GetOptions{})
if err != nil {
return err
}
if cond := meta.FindStatusCondition(actual.Status.Conditions, "WorkDriverConfigSecretSynced"); cond != nil {
return fmt.Errorf("should remove WorkDriverConfigSecretSynced condition")
}
return nil
}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())

gomega.Eventually(func() error {
actual, err := hostedKubeClient.AppsV1().Deployments(hubNamespaceHosted).Get(context.Background(),
hubWorkControllerDeployment, metav1.GetOptions{})
Expand All @@ -587,6 +624,10 @@ var _ = ginkgo.Describe("ClusterManager Hosted Mode", func() {
}
return errors.IsNotFound(err)
}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue())

err = hostedKubeClient.CoreV1().Secrets("default").Delete(context.Background(),
helpers.WorkDriverConfig, metav1.DeleteOptions{})
gomega.Expect(err).NotTo(gomega.HaveOccurred())
})

ginkgo.It("should have expected resource created/deleted successfully when feature gates AddOnManager enabled/disabled", func() {
Expand Down
41 changes: 41 additions & 0 deletions test/integration/operator/clustermanager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/rest"
"k8s.io/client-go/util/cert"
Expand Down Expand Up @@ -478,6 +479,18 @@ var _ = ginkgo.Describe("ClusterManager Default Mode", func() {
return err
}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil())

gomega.Eventually(func() error {
actual, err := operatorClient.OperatorV1().ClusterManagers().Get(
context.Background(), clusterManagerName, metav1.GetOptions{})
if err != nil {
return err
}
if !meta.IsStatusConditionFalse(actual.Status.Conditions, "WorkDriverConfigSecretSynced") {
return fmt.Errorf("should get WorkDriverConfigSecretSynced condition false")
}
return nil
}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())

_, err := kubeClient.CoreV1().Secrets("default").Create(context.TODO(), &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: helpers.WorkDriverConfig,
Expand All @@ -489,6 +502,18 @@ var _ = ginkgo.Describe("ClusterManager Default Mode", func() {
}, metav1.CreateOptions{})
gomega.Expect(err).NotTo(gomega.HaveOccurred())

gomega.Eventually(func() error {
actual, err := operatorClient.OperatorV1().ClusterManagers().Get(
context.Background(), clusterManagerName, metav1.GetOptions{})
if err != nil {
return err
}
if !meta.IsStatusConditionTrue(actual.Status.Conditions, "WorkDriverConfigSecretSynced") {
return fmt.Errorf("should get WorkDriverConfigSecretSynced condition true")
}
return nil
}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())

gomega.Eventually(func() error {
actual, err := kubeClient.AppsV1().Deployments(hubNamespace).Get(context.Background(),
hubWorkControllerDeployment, metav1.GetOptions{})
Expand Down Expand Up @@ -541,6 +566,18 @@ var _ = ginkgo.Describe("ClusterManager Default Mode", func() {
return err
}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil())

gomega.Eventually(func() error {
actual, err := operatorClient.OperatorV1().ClusterManagers().Get(
context.Background(), clusterManagerName, metav1.GetOptions{})
if err != nil {
return err
}
if cond := meta.FindStatusCondition(actual.Status.Conditions, "WorkDriverConfigSecretSynced"); cond != nil {
return fmt.Errorf("should remove WorkDriverConfigSecretSynced condition")
}
return nil
}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred())

gomega.Eventually(func() error {
actual, err := kubeClient.AppsV1().Deployments(hubNamespace).Get(context.Background(),
hubWorkControllerDeployment, metav1.GetOptions{})
Expand All @@ -563,6 +600,10 @@ var _ = ginkgo.Describe("ClusterManager Default Mode", func() {
}
return errors.IsNotFound(err)
}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue())

err = kubeClient.CoreV1().Secrets("default").Delete(context.Background(),
helpers.WorkDriverConfig, metav1.DeleteOptions{})
gomega.Expect(err).NotTo(gomega.HaveOccurred())
})

ginkgo.It("should have expected resource created/deleted successfully when feature gates AddOnManager enabled/disabled", func() {
Expand Down

0 comments on commit 8e56daf

Please sign in to comment.