Watching for Kyma Custom Resource #7

mvshao · 2023-05-10T07:57:31Z

Description

Changes proposed in this pull request:

implemented first version of compass-manager
implemented logic for watching Kyma CR
implemented event filter

Related issue(s)
kyma-project/kyma#9

kyma-bot · 2023-05-10T07:57:36Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

VOID404 · 2023-05-11T13:35:08Z

/test all

mvshao · 2023-05-15T10:17:29Z

/test all

mvshao · 2023-05-19T14:35:58Z

/test all

mvshao · 2023-05-19T14:42:30Z

/test all

controllers/compassmanager_controller.go

akgalwas · 2023-05-31T09:48:47Z

controllers/compassmanager_controller.go

+
+	cm.Log.Infof("reconciliation triggered for resource named: %s", req.Name)
+	kymaName := req.Name
+	kubeconfigSecretName := "kubeconfig-" + req.Name


I don't think it isa good idea to build the secret name. The more bulletproof solution would be to find the secret based on a label.

akgalwas · 2023-06-05T10:37:36Z

api/v1beta1/compassmanager_types.go

@@ -0,0 +1,48 @@
+package v1beta1


Compass Manager doesn't need to define its own CRD.

akgalwas · 2023-06-05T10:38:24Z

config/crd/bases/operator.kyma-project.io_compassmanagers.yaml

@@ -0,0 +1,50 @@
+---


Not needed, as Compass Manager doesn't need have its own CRD.

akgalwas · 2023-06-05T10:38:34Z

api/v1beta1/groupversion_info.go

@@ -0,0 +1,20 @@
+// Package v1beta1 contains API Schema definitions for the operator v1beta1 API group


Not needed, as Compass Manager doesn't need have its own CRD.

akgalwas · 2023-06-05T10:40:27Z

config/crd/patches/webhook_in_compassmanagers.yaml

@@ -0,0 +1,16 @@
+# The following patch enables a conversion webhook for the CRD


Is it needed? I think not, as Compass Manager doesn't need have its own CRD.

akgalwas · 2023-06-05T10:41:23Z

config/rbac/compassmanager_editor_role.yaml

@@ -0,0 +1,31 @@
+# permissions for end users to edit compassmanagers.


I think it is not needed

akgalwas · 2023-06-06T05:51:08Z

controllers/compassmanager_controller.go

+		UpdateFunc: func(e event.UpdateEvent) bool {
+			return cm.checkUpdateCompassLabel(e.ObjectNew, e.ObjectOld)
+		},
+		GenericFunc: func(e event.GenericEvent) bool {


I'm not sure we need to provide GenericFunc.

akgalwas · 2023-06-06T05:52:38Z

controllers/compassmanager_controller.go

+		LabelSelector: labelSelector,
+	})
+	if err != nil {
+		cm.Log.Infof("%v", err)


Logging errors with Info level is misleading.

akgalwas · 2023-06-06T05:54:34Z

controllers/compassmanager_controller.go

+	instance := &kyma.Kyma{}
+	err := cm.Client.Get(context.Background(), objKey, instance)
+	if err != nil {
+		if errors.IsNotFound(err) {


I don't think we need to distinguish error type here.

akgalwas · 2023-06-06T06:03:45Z

controllers/compassmanager_controller.go

+func (cm *CompassManagerReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	fieldSelectorPredicate := predicate.Funcs{
+		CreateFunc: func(e event.CreateEvent) bool {
+			return cm.checkCompassLabel(e.Object)


checkCompassLabel is not descriptive enough.

akgalwas · 2023-06-06T06:05:46Z

controllers/compassmanager_controller.go

+
+	err = cm.Client.Update(context.Background(), instance)
+	if err != nil {
+		log.Infof("%v, %s", err, " failed to update Kyma Custom Resource")


Logging errors with Info level is misleading.

mvshao · 2023-06-06T07:44:22Z

@akgalwas Thank you for your review and suggestions! 😄

mvshao · 2023-06-12T12:18:32Z

/check-cla

controllers/compassmanager_controller_test.go

akgalwas · 2023-06-16T09:11:56Z

controllers/compassmanager_controller_test.go

+				h.shouldCreateKyma(testSuiteKyma.Name, testSuiteKyma)
+
+				// then
+				h.shouldCheckCompassLabel(testSuiteKyma.Name, testSuiteKyma.Namespace, true)


This assertion is not correct. We should wait for some time, and periodically ensure the predicate (label not set) is met. This is not how this code works. As it uses Eventually the predicate is true the first time it is checked before timeout is passed. I would suggest Consistently directive instead on Eventually.

controllers/compassmanager_controller_test.go

kyma-bot · 2023-06-21T11:42:23Z

@mvshao: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-compass-manager-lint	`623034c`	link	false	`/test pull-compass-manager-lint`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

mvshao added 4 commits May 5, 2023 10:49

Watching resource Kyma CR, temp image name

a96042e

First version of event filters

513f358

Working watching Kubeconfig field

a8fcabb

Prepare to present two types of watching resources

bc05c0b

mvshao added the area/application-connector Issues or PRs related to application connectivity label May 10, 2023

kyma-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 10, 2023

kyma-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 10, 2023

mvshao changed the title ~~[WIP] Watching for Kyma Custom Resource~~ [WIP] Watching for Kyma Custom Resource - flow with For() method May 10, 2023

mvshao changed the title ~~[WIP] Watching for Kyma Custom Resource - flow with For() method~~ [WIP] Watching for Kyma Custom Resource May 10, 2023

mvshao mentioned this pull request Jul 11, 2023

High availability for connecting Kyma runtime with Compass (UCL) #107

Open

Fix invalid format in fmt.Sprintf

28c5a68

mvshao added 3 commits May 15, 2023 10:17

Code cleanup

2d328d5

First round of unit tests for controller

5422866

Working test cluster, need to implement tests

3cdb836

Envtests for reconcilation loop

1eefe3a

Fix Makefile

d886495

mvshao changed the title ~~[WIP] Watching for Kyma Custom Resource~~ Watching for Kyma Custom Resource May 22, 2023

mvshao marked this pull request as ready for review May 22, 2023 07:13

mvshao requested a review from a team as a code owner May 22, 2023 07:13

kyma-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 22, 2023

mvshao added 3 commits May 22, 2023 09:30

Fix linting errors

c8669ad

Change the default Docker image path

b2b479e

Fix imports

bf79026

akgalwas reviewed May 23, 2023

View reviewed changes

controllers/compassmanager_controller.go Show resolved Hide resolved

Labeling Kyma CR with compass-id

bb97a69

mvshao added 5 commits May 29, 2023 13:51

Skip reconcilation if not signicifant filed in CR is chaned

8196d8c

Pass the custom interface

ace9b9a

Provide tests for reconcilation loop

f0f3c0a

Resolve vulnerability CVE-2023-2253

0ecc24c

Fix major lint errors

072ab87

akgalwas reviewed May 31, 2023

View reviewed changes

mvshao added 2 commits June 1, 2023 15:28

Implement BDD Ginkgo tests

9e5b6bf

Fix Sprintf error

eab88b4

akgalwas requested changes Jun 6, 2023

View reviewed changes

kyma-bot assigned akgalwas Jun 6, 2023

akgalwas and others added 5 commits June 6, 2023 14:10

Proposed refactoring in the controller.

7fa9906

Proposed refactoring in the controller.

ffdc016

Fix ginkgo tests

84f7ab2

Deterministic env tests

2d56740

Reconciler returns errror with requeue

f23343f

akgalwas requested changes Jun 16, 2023

View reviewed changes

akgalwas and others added 6 commits June 16, 2023 13:21

Proposed refactoring in the controller's tests.

947a45a

Ginkgo test refactor and cleanup

e901238

Describe table for test cases

e7919c3

Remove compassManager CRD

d244c4d

Cleanup in test and in CRD's

b436cad

Proposal for simplify predicate passing

623034c

akgalwas approved these changes Jun 21, 2023

View reviewed changes

kyma-bot added the lgtm Looks good to me! label Jun 21, 2023

kyma-bot merged commit ee54e54 into kyma-project:main Jun 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Watching for Kyma Custom Resource #7

Watching for Kyma Custom Resource #7

mvshao commented May 10, 2023 •

edited

Loading

kyma-bot commented May 10, 2023

VOID404 commented May 11, 2023

mvshao commented May 15, 2023

mvshao commented May 19, 2023

mvshao commented May 19, 2023

akgalwas May 31, 2023

akgalwas Jun 5, 2023

akgalwas Jun 5, 2023

akgalwas Jun 5, 2023

akgalwas Jun 5, 2023

akgalwas Jun 5, 2023

akgalwas Jun 6, 2023

akgalwas Jun 6, 2023

akgalwas Jun 6, 2023

akgalwas Jun 6, 2023

akgalwas Jun 6, 2023

mvshao commented Jun 6, 2023 •

edited

Loading

mvshao commented Jun 12, 2023

akgalwas Jun 16, 2023

kyma-bot commented Jun 21, 2023

		@@ -0,0 +1,20 @@
		// Package v1beta1 contains API Schema definitions for the operator v1beta1 API group

		@@ -0,0 +1,16 @@
		# The following patch enables a conversion webhook for the CRD

		@@ -0,0 +1,31 @@
		# permissions for end users to edit compassmanagers.

Watching for Kyma Custom Resource #7

Watching for Kyma Custom Resource #7

Conversation

mvshao commented May 10, 2023 • edited Loading

kyma-bot commented May 10, 2023

VOID404 commented May 11, 2023

mvshao commented May 15, 2023

mvshao commented May 19, 2023

mvshao commented May 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvshao commented Jun 6, 2023 • edited Loading

mvshao commented Jun 12, 2023

Choose a reason for hiding this comment

kyma-bot commented Jun 21, 2023

mvshao commented May 10, 2023 •

edited

Loading

mvshao commented Jun 6, 2023 •

edited

Loading