Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compass-id-for-migration #32

Merged
merged 5 commits into from
Oct 18, 2023
Merged

compass-id-for-migration #32

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5827832
Write test for `compass-id-for-migration`
VOID404 Oct 10, 2023
3251e18
Handle `compass-id-for-migration`
VOID404 Oct 10, 2023
e638bb6
Apply lint suggestions
VOID404 Oct 11, 2023
b746510
Apply review suggestions
VOID404 Oct 16, 2023
dc7ae90
Bump libraries
VOID404 Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/v1beta1/compassmanagermapping_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ type CompassManagerMapping struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec CompassManagerMappingSpec `json:"spec,omitempty"`
Status CompassManagerMappingStatus `json:"status,omitempty"`
Spec CompassManagerMappingSpec `json:"spec,omitempty"`
}

//+kubebuilder:object:root=true
Expand Down
2 changes: 1 addition & 1 deletion config/manager/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
images:
- name: controller
newName: mvshao/compass-manager
newTag: latest
newName: compass-manager
1 change: 1 addition & 0 deletions config/rbac/role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ rules:
- delete
- get
- list
- watch
- apiGroups:
- operator.kyma-project.io
resources:
Expand Down
86 changes: 61 additions & 25 deletions controllers/compassmanager_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,28 @@ import (
)

const (
KymaNameLabel = "operator.kyma-project.io/kyma-name"
BrokerPlanIDLabel = "kyma-project.io/broker-plan-id"
BrokerPlanNameLabel = "kyma-project.io/broker-plan-name"
GlobalAccountIDLabel = "kyma-project.io/global-account-id"
BrokerInstanceIDLabel = "kyma-project.io/instance-id"
ShootNameLabel = "kyma-project.io/shoot-name"
SubaccountIDLabel = "kyma-project.io/subaccount-id"
ComppassIDLabel = "kyma-project.io/compass-runtime-id"
ManagedByLabel = "operator.kyma-project.io/managed-by"
AnnotationIDForMigration = "compass-runtime-id-for-migration"

LabelBrokerInstanceID = "kyma-project.io/instance-id"
LabelBrokerPlanID = "kyma-project.io/broker-plan-id"
LabelBrokerPlanName = "kyma-project.io/broker-plan-name"
LabelComppassID = "kyma-project.io/compass-runtime-id"
LabelGlobalAccountID = "kyma-project.io/global-account-id"
LabelKymaName = "operator.kyma-project.io/kyma-name"
LabelManagedBy = "operator.kyma-project.io/managed-by"
LabelShootName = "kyma-project.io/shoot-name"
LabelSubaccountID = "kyma-project.io/subaccount-id"

ApplicationConnectorModuleName = "application-connector-module"

// KubeconfigKey is the name of the key in the secret storing cluster credentials.
// The secret is created by KEB: https://github.com/kyma-project/control-plane/blob/main/components/kyma-environment-broker/internal/process/steps/lifecycle_manager_kubeconfig.go
KubeconfigKey = "config"
)

//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch
//+kubebuilder:rbac:groups=operator.kyma-project.io,resources=kymas,verbs=get;list;watch
//+kubebuilder:rbac:groups=operator.kyma-project.io,resources=compassmanagermappings,verbs=create;get;list;delete
//+kubebuilder:rbac:groups=operator.kyma-project.io,resources=compassmanagermappings,verbs=create;get;list;delete;watch
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compass Manager also needs the update method in RBAC to perform Upsert could you add it?

//+kubebuilder:rbac:groups=operator.kyma-project.io,resources=kymas/status,verbs=get
//+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch

Expand Down Expand Up @@ -106,7 +110,23 @@ func (cm *CompassManagerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{RequeueAfter: cm.requeueTime}, err
}

kymaAnnotations, err := cm.getKymaAnnotations(req.NamespacedName)
if err != nil {
cm.Log.Warnf("Failed to obtain annotations from Kyma resource %s: %v.", req.Name, err)
return ctrl.Result{RequeueAfter: cm.requeueTime}, err
Copy link
Contributor

@mvshao mvshao Oct 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RequeueAfter doesn't work if error was returned from Reconcile. Consider this flow

  • if the error is from the k8s client-go method e.g. Get, List, Update the controller should set itself to Error state
  • if the error results from any other component such as a problem with connection to Compass Director etc, the request should be requeueed in RequeueAfter with nil error. Before return we should log.Warn with message.

}

compassRuntimeID, err := cm.getRuntimeIDFromCompassMapping(req.Name, req.Namespace)

if migrationCompassRuntimeID, ok := kymaAnnotations[AnnotationIDForMigration]; compassRuntimeID == "" && ok {
cm.Log.Infof("Configuring compass for already registered Kyma resource %s.", req.Name)
cmerr := cm.upsertCompassMappingResource(migrationCompassRuntimeID, req.Namespace, kymaLabels)
if cmerr != nil {
return ctrl.Result{RequeueAfter: cm.requeueTime}, errors.Wrap(cmerr, "failed to create Compass Manager Mapping for an already registered Kyma")
}
return ctrl.Result{}, nil
}

if err != nil {
return ctrl.Result{RequeueAfter: cm.requeueTime}, err
}
Expand Down Expand Up @@ -144,7 +164,7 @@ func (cm *CompassManagerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
func (cm *CompassManagerReconciler) getKubeconfig(kymaName string) (string, error) {
secretList := &corev1.SecretList{}
labelSelector := labels.SelectorFromSet(map[string]string{
KymaNameLabel: kymaName,
LabelKymaName: kymaName,
})

err := cm.Client.List(context.Background(), secretList, &client.ListOptions{
Expand All @@ -163,6 +183,22 @@ func (cm *CompassManagerReconciler) getKubeconfig(kymaName string) (string, erro
return string(secret.Data[KubeconfigKey]), nil
}

func (cm *CompassManagerReconciler) getKymaAnnotations(objKey types.NamespacedName) (map[string]string, error) {
instance := &kyma.Kyma{}

err := cm.Client.Get(context.Background(), objKey, instance)
if err != nil {
return nil, err
}

a := instance.GetAnnotations()
if a == nil {
a = make(map[string]string)
}

return a, nil
}

func (cm *CompassManagerReconciler) getKymaLabels(objKey types.NamespacedName) (map[string]string, error) {
instance := &kyma.Kyma{}

Expand All @@ -180,17 +216,17 @@ func (cm *CompassManagerReconciler) getKymaLabels(objKey types.NamespacedName) (
}

func (cm *CompassManagerReconciler) upsertCompassMappingResource(compassRuntimeID, namespace string, kymaLabels map[string]string) error {
kymaName := kymaLabels[KymaNameLabel]
kymaName := kymaLabels[LabelKymaName]
compassMapping := &v1beta1.CompassManagerMapping{}
compassMapping.Name = kymaName
compassMapping.Namespace = namespace

compassMappingLabels := make(map[string]string)
compassMappingLabels[KymaNameLabel] = kymaLabels[KymaNameLabel]
compassMappingLabels[ComppassIDLabel] = compassRuntimeID
compassMappingLabels[GlobalAccountIDLabel] = kymaLabels[GlobalAccountIDLabel]
compassMappingLabels[SubaccountIDLabel] = kymaLabels[SubaccountIDLabel]
compassMappingLabels[ManagedByLabel] = "compass-manager"
compassMappingLabels[LabelKymaName] = kymaLabels[LabelKymaName]
compassMappingLabels[LabelComppassID] = compassRuntimeID
compassMappingLabels[LabelGlobalAccountID] = kymaLabels[LabelGlobalAccountID]
compassMappingLabels[LabelSubaccountID] = kymaLabels[LabelSubaccountID]
compassMappingLabels[LabelManagedBy] = "compass-manager"

compassMapping.SetLabels(compassMappingLabels)

Expand All @@ -215,7 +251,7 @@ func (cm *CompassManagerReconciler) upsertCompassMappingResource(compassRuntimeI
func (cm *CompassManagerReconciler) getRuntimeIDFromCompassMapping(kymaName string, namespace string) (string, error) {
mappingList := &v1beta1.CompassManagerMappingList{}
labelSelector := labels.SelectorFromSet(map[string]string{
KymaNameLabel: kymaName,
LabelKymaName: kymaName,
})

err := cm.Client.List(context.Background(), mappingList, &client.ListOptions{
Expand All @@ -231,7 +267,7 @@ func (cm *CompassManagerReconciler) getRuntimeIDFromCompassMapping(kymaName stri
return "", nil
}

return mappingList.Items[0].GetLabels()[ComppassIDLabel], nil
return mappingList.Items[0].GetLabels()[LabelComppassID], nil
}

// SetupWithManager sets up the controller with the Manager.
Expand Down Expand Up @@ -287,12 +323,12 @@ func (cm *CompassManagerReconciler) needsToBeReconciled(obj runtime.Object) bool
func createCompassRuntimeLabels(kymaLabels map[string]string) map[string]interface{} {
runtimeLabels := make(map[string]interface{})
runtimeLabels["director_connection_managed_by"] = "compass-manager"
runtimeLabels["broker_instance_id"] = kymaLabels[BrokerInstanceIDLabel]
runtimeLabels["gardenerClusterName"] = kymaLabels[ShootNameLabel]
runtimeLabels["subaccount_id"] = kymaLabels[SubaccountIDLabel]
runtimeLabels["global_account_id"] = kymaLabels[GlobalAccountIDLabel]
runtimeLabels["broker_plan_id"] = kymaLabels[BrokerPlanIDLabel]
runtimeLabels["broker_plan_name"] = kymaLabels[BrokerPlanNameLabel]
runtimeLabels["broker_instance_id"] = kymaLabels[LabelBrokerInstanceID]
runtimeLabels["gardenerClusterName"] = kymaLabels[LabelShootName]
runtimeLabels["subaccount_id"] = kymaLabels[LabelSubaccountID]
runtimeLabels["global_account_id"] = kymaLabels[LabelGlobalAccountID]
runtimeLabels["broker_plan_id"] = kymaLabels[LabelBrokerPlanID]
runtimeLabels["broker_plan_name"] = kymaLabels[LabelBrokerPlanName]

return runtimeLabels
}
46 changes: 34 additions & 12 deletions controllers/compassmanager_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,27 @@ var _ = Describe("Compass Manager controller", func() {
kymaCustomResourceLabels := make(map[string]string)
kymaCustomResourceLabels["operator.kyma-project.io/managed-by"] = "lifecycle-manager"

It("handles `compass-runtime-id-for-migration`", func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When creating a test case we follow this pattern: Describe -> Context -> It / DescribeTable. Could you adjust this test case to the pattern? You can check how the other tests are described.

const kymaName = "preregistered"
const preRegisteredID = "preregistered-id"

secret := createCredentialsSecret(kymaName, kymaCustomResourceNamespace)
Expect(k8sClient.Create(context.Background(), &secret)).To(Succeed())

By("Create Kyma Resource")
kymaCR := createKymaResource(kymaName)
kymaCR.Annotations["compass-runtime-id-for-migration"] = preRegisteredID
Expect(k8sClient.Create(context.Background(), &kymaCR)).To(Succeed())

Eventually(func() string {
label, err := getCompassMappingCompassID(kymaCR.Name)
if err != nil {
return err.Error()
}
return label
}, clientTimeout, clientInterval).Should(Equal(preRegisteredID))
})

Context("Secret with Kubeconfig is correctly created, and assigned to Kyma resource", func() {
DescribeTable("Register Runtime in the Director, and configure Compass Runtime Agent", func(kymaName string) {
By("Create secret with credentials")
Expand All @@ -38,7 +59,7 @@ var _ = Describe("Compass Manager controller", func() {
Expect(k8sClient.Create(context.Background(), &kymaCR)).To(Succeed())

Eventually(func() bool {
label, err := getCompassMappingLabel(kymaCR.Name, ComppassIDLabel, kymaCustomResourceNamespace)
label, err := getCompassMappingCompassID(kymaCR.Name)

return err == nil && label != ""
}, clientTimeout, clientInterval).Should(BeTrue())
Expand All @@ -57,7 +78,7 @@ var _ = Describe("Compass Manager controller", func() {
Expect(k8sClient.Create(context.Background(), &kymaCR)).To(Succeed())

Consistently(func() bool {
label, err := getCompassMappingLabel(kymaCR.Name, ComppassIDLabel, kymaCustomResourceNamespace)
label, err := getCompassMappingCompassID(kymaCR.Name)

return errors.IsNotFound(err) && label == ""
}, clientTimeout, clientInterval).Should(BeTrue())
Expand All @@ -67,7 +88,7 @@ var _ = Describe("Compass Manager controller", func() {
Expect(k8sClient.Create(context.Background(), &secret)).To(Succeed())

Eventually(func() bool {
label, err := getCompassMappingLabel(kymaCR.Name, ComppassIDLabel, kymaCustomResourceNamespace)
label, err := getCompassMappingCompassID(kymaCR.Name)

return err == nil && label != ""
}, clientTimeout, clientInterval).Should(BeTrue())
Expand Down Expand Up @@ -121,9 +142,9 @@ func createNamespace(name string) error {

func createKymaResource(name string) kyma.Kyma {
kymaCustomResourceLabels := make(map[string]string)
kymaCustomResourceLabels[GlobalAccountIDLabel] = "globalAccount"
kymaCustomResourceLabels[ShootNameLabel] = name
kymaCustomResourceLabels[KymaNameLabel] = name
kymaCustomResourceLabels[LabelGlobalAccountID] = "globalAccount"
kymaCustomResourceLabels[LabelShootName] = name
kymaCustomResourceLabels[LabelKymaName] = name

kymaModules := make([]kyma.Module, 1)
kymaModules[0].Name = ApplicationConnectorModuleName
Expand All @@ -134,9 +155,10 @@ func createKymaResource(name string) kyma.Kyma {
APIVersion: kymaCustomResourceAPIVersion,
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: kymaCustomResourceNamespace,
Labels: kymaCustomResourceLabels,
Name: name,
Namespace: kymaCustomResourceNamespace,
Labels: kymaCustomResourceLabels,
Annotations: make(map[string]string),
},
Spec: kyma.KymaSpec{
Channel: "regular",
Expand All @@ -160,17 +182,17 @@ func createCredentialsSecret(kymaName, namespace string) corev1.Secret {
}
}

func getCompassMappingLabel(kymaName, labelName, namespace string) (string, error) {
func getCompassMappingCompassID(kymaName string) (string, error) {
var obj v1beta1.CompassManagerMapping
key := types.NamespacedName{Name: kymaName, Namespace: namespace}
key := types.NamespacedName{Name: kymaName, Namespace: kymaCustomResourceNamespace}

err := cm.Client.Get(context.Background(), key, &obj)
if err != nil {
return "", err
}

labels := obj.GetLabels()
return labels[labelName], nil
return labels[LabelComppassID], nil
}

// Feature (refreshing token) is implemented but according to our discussions, it will be a part of another PR
Expand Down
17 changes: 12 additions & 5 deletions controllers/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,6 @@ var _ = AfterSuite(func() {
})

func prepareMockFunctions(c *mocks.Configurator, r *mocks.Registrator) {
compassLabelsAllGood := createCompassRuntimeLabels(map[string]string{ShootNameLabel: "all-good", GlobalAccountIDLabel: "globalAccount"})
compassLabelsConfigureFails := createCompassRuntimeLabels(map[string]string{ShootNameLabel: "configure-fails", GlobalAccountIDLabel: "globalAccount"})
compassLabelsRegistrationFails := createCompassRuntimeLabels(map[string]string{ShootNameLabel: "registration-fails", GlobalAccountIDLabel: "globalAccount"})
compassLabelsEmptyKubeconfig := createCompassRuntimeLabels(map[string]string{ShootNameLabel: "empty-kubeconfig", GlobalAccountIDLabel: "globalAccount"})

// Feature (refreshing token) is implemented but according to our discussions, it will be a part of another PR
// compassLabelsRefreshToken := createCompassRuntimeLabels(map[string]string{ShootNameLabel: "refresh-token", GlobalAccountIDLabel: "globalAccount"})
// refreshedToken := graphql.OneTimeTokenForRuntimeExt{
Expand All @@ -128,19 +123,31 @@ func prepareMockFunctions(c *mocks.Configurator, r *mocks.Registrator) {
// RawEncoded: "rawEncodedToken",
//}

// It handles `compass-runtime-id-for-migration`
compassLabelsRegistered := createCompassRuntimeLabels(map[string]string{LabelShootName: "preregistered", LabelGlobalAccountID: "globalAccount"})
r.On("RegisterInCompass", compassLabelsRegistered).Return("id-preregistered-incorrect", nil)
// failing test case
c.On("ConfigureCompassRuntimeAgent", "kubeconfig-data-preregistered", "id-preregistered-incorrect").Return(nil)
// succeeding test case
c.On("ConfigureCompassRuntimeAgent", "id-preregistered-incorrect", "preregistered").Return(errors.New("This shouldn't be called"))

compassLabelsAllGood := createCompassRuntimeLabels(map[string]string{LabelShootName: "all-good", LabelGlobalAccountID: "globalAccount"})
r.On("RegisterInCompass", compassLabelsAllGood).Return("id-all-good", nil)
c.On("ConfigureCompassRuntimeAgent", "kubeconfig-data-all-good", "id-all-good").Return(nil)

compassLabelsConfigureFails := createCompassRuntimeLabels(map[string]string{LabelShootName: "configure-fails", LabelGlobalAccountID: "globalAccount"})
// The first call to ConfigureRuntimeAgent fails, but the second is successful
r.On("RegisterInCompass", compassLabelsConfigureFails).Return("id-configure-fails", nil)
c.On("ConfigureCompassRuntimeAgent", "kubeconfig-data-configure-fails", "id-configure-fails").Return(errors.New("error during configuration of Compass Runtime Agent CR")).Once()
c.On("ConfigureCompassRuntimeAgent", "kubeconfig-data-configure-fails", "id-configure-fails").Return(nil).Once()

compassLabelsRegistrationFails := createCompassRuntimeLabels(map[string]string{LabelShootName: "registration-fails", LabelGlobalAccountID: "globalAccount"})
// The first call to RegisterInCompass fails, but the second is successful.
r.On("RegisterInCompass", compassLabelsRegistrationFails).Return("", errors.New("error during registration")).Once()
r.On("RegisterInCompass", compassLabelsRegistrationFails).Return("registration-fails", nil).Once()
c.On("ConfigureCompassRuntimeAgent", "kubeconfig-data-registration-fails", "registration-fails").Return(nil)

compassLabelsEmptyKubeconfig := createCompassRuntimeLabels(map[string]string{LabelShootName: "empty-kubeconfig", LabelGlobalAccountID: "globalAccount"})
r.On("RegisterInCompass", compassLabelsEmptyKubeconfig).Return("id-empty-kubeconfig", nil)
c.On("ConfigureCompassRuntimeAgent", "kubeconfig-data-empty-kubeconfig", "id-empty-kubeconfig").Return(nil)

Expand Down