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 Apr 2, 2024
1 parent 9959501 commit 6fdf148
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 79 deletions.
62 changes: 2 additions & 60 deletions pkg/operator/helpers/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -725,66 +725,8 @@ 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,
clusterManagerNamespace string,
) error {
if _, err := operatorClient.CoreV1().Namespaces().Get(ctx, clusterManagerNamespace, metav1.GetOptions{}); err != nil {
if errors.IsNotFound(err) {
return nil
}
return err
}
err := operatorClient.CoreV1().Secrets(clusterManagerNamespace).Delete(ctx, WorkDriverConfig, metav1.DeleteOptions{})
if err != nil && errors.IsNotFound(err) {
return nil
}

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
46 changes: 43 additions & 3 deletions ...perators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const (
clusterManagerFinalizer = "operator.open-cluster-management.io/cluster-manager-cleanup"
clusterManagerApplied = "Applied"
clusterManagerProgressing = "Progressing"
secretSyncedConditionType = "SecretSynced"

defaultWebhookPort = int32(9443)
clusterManagerReSyncTime = 5 * time.Second
Expand Down Expand Up @@ -218,12 +219,51 @@ func (n *clusterManagerController) sync(ctx context.Context, controllerContext f

// 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 {
operatorNamespace, err := helpers.GetOperatorNamespace()
if err != nil {
return err
}
} else {
if err := helpers.RemoveWorkConfigSecret(ctx, n.operatorKubeClient, clusterManagerNamespace); err != nil {
// 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: secretSyncedConditionType,
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: secretSyncedConditionType,
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
}
}
}
}

Expand Down
62 changes: 62 additions & 0 deletions ...ors/clustermanager/controllers/clustermanagercontroller/clustermanager_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,68 @@ func ensureObject(t *testing.T, object runtime.Object, hubCore *operatorapiv1.Cl
}
}

func TestSyncSecret(t *testing.T) {
operatorNamespace := "default"
t.Setenv("POD_NAMESPACE", operatorNamespace)
clusterManager := newClusterManager("testhub")
clusterManager.Spec.WorkConfiguration.WorkDriver = operatorapiv1.WorkDriverTypeGrpc
tc := newTestController(t, clusterManager)
clusterManagerNamespace := helpers.ClusterManagerNamespace(clusterManager.Name, clusterManager.Spec.DeployOption.Mode)
setup(t, tc, nil)

syncContext := testingcommon.NewFakeSyncContext(t, "testhub")

err := tc.clusterManagerController.sync(ctx, syncContext)
if err == nil {
t.Fatalf("Expected error when sync, %v", err)
}

clusterManager, err = tc.operatorClient.OperatorV1().ClusterManagers().Get(ctx, clusterManager.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("Failed to get cluster manager: %v", err)
}

if !meta.IsStatusConditionFalse(clusterManager.Status.Conditions, secretSyncedConditionType) {
t.Fatalf("Expected secretSyncedConditionType to be false")
}

workDriverConfig := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "work-driver-config",
},
Data: map[string][]byte{
"config.yaml": []byte("url: grpc.example.com:8443"),
},
}

if _, err = tc.managementKubeClient.CoreV1().Secrets(operatorNamespace).Create(ctx, workDriverConfig, metav1.CreateOptions{}); err != nil {
t.Fatalf("Failed to create work driver config secret: %v", err)
}

err = tc.clusterManagerController.sync(ctx, syncContext)
if err != nil {
t.Fatalf("Expected no error when sync, %v", err)
}

clusterManager, err = tc.operatorClient.OperatorV1().ClusterManagers().Get(ctx, clusterManager.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("Failed to get cluster manager: %v", err)
}

if !meta.IsStatusConditionTrue(clusterManager.Status.Conditions, secretSyncedConditionType) {
t.Fatalf("Expected secretSyncedConditionType to be true")
}

syncedSecret, err := tc.managementKubeClient.CoreV1().Secrets(clusterManagerNamespace).Get(ctx, "work-driver-config", metav1.GetOptions{})
if err != nil {
t.Fatalf("Failed to get synced work driver config secret: %v", err)
}

if string(syncedSecret.Data["config.yaml"]) != "url: grpc.example.com:8443" {
t.Fatalf("Expected secret data to be url: grpc.example.com:8443")
}
}

// TestSyncDeploy tests sync manifests of hub component
func TestSyncDeploy(t *testing.T) {
clusterManager := newClusterManager("testhub")
Expand Down
36 changes: 28 additions & 8 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, "SecretSynced") {
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, "SecretSynced") {
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 @@ -579,14 +604,9 @@ var _ = ginkgo.Describe("ClusterManager Hosted Mode", func() {
return nil
}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil())

gomega.Eventually(func() bool {
_, err := hostedKubeClient.CoreV1().Secrets(hubNamespaceHosted).Get(context.Background(),
helpers.WorkDriverConfig, metav1.GetOptions{})
if err == nil {
return false
}
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
36 changes: 28 additions & 8 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, "SecretSynced") {
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, "SecretSynced") {
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 @@ -555,14 +580,9 @@ var _ = ginkgo.Describe("ClusterManager Default Mode", func() {
return nil
}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil())

gomega.Eventually(func() bool {
_, err := kubeClient.CoreV1().Secrets(hubNamespace).Get(context.Background(),
helpers.WorkDriverConfig, metav1.GetOptions{})
if err == nil {
return false
}
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 6fdf148

Please sign in to comment.