From b7809fa36224ae73c1a710dfdcfb7a16ec820dc6 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Thu, 20 Mar 2025 20:25:07 +0100 Subject: [PATCH 1/4] Update to the latest toolchain-common --- go.mod | 5 +++-- go.sum | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 828d1ab2c..af13055a2 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/codeready-toolchain/host-operator require ( cloud.google.com/go/recaptchaenterprise/v2 v2.13.0 github.com/codeready-toolchain/api v0.0.0-20250313170542-4e3c4147cb80 - github.com/codeready-toolchain/toolchain-common v0.0.0-20250313203311-0bce6563576f + github.com/codeready-toolchain/toolchain-common v0.0.0-20250430091615-95cf792ac171 github.com/davecgh/go-spew v1.1.1 // indirect github.com/go-bindata/go-bindata v3.1.2+incompatible github.com/go-logr/logr v1.4.1 @@ -31,6 +31,8 @@ require ( sigs.k8s.io/controller-runtime v0.18.4 ) +require k8s.io/kubectl v0.30.1 + require ( cloud.google.com/go/auth v0.3.0 // indirect cloud.google.com/go/auth/oauth2adapt v0.2.2 // indirect @@ -117,7 +119,6 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/cli-runtime v0.30.1 // indirect k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect - k8s.io/kubectl v0.30.1 // indirect k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/kustomize/api v0.13.5-0.20230601165947-6ce0bf390ce3 // indirect diff --git a/go.sum b/go.sum index c6a7d4ce8..194b71666 100644 --- a/go.sum +++ b/go.sum @@ -38,8 +38,8 @@ github.com/cloudflare/circl v1.3.7/go.mod h1:sRTcRWXGLrKw6yIGJ+l7amYJFfAXbZG0kBS github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/codeready-toolchain/api v0.0.0-20250313170542-4e3c4147cb80 h1:wh41dZ7VkyClQWiMJbMypku6xTxtxNZTgQJUAcodgUo= github.com/codeready-toolchain/api v0.0.0-20250313170542-4e3c4147cb80/go.mod h1:shTTQ+bYWinbdF/oK+ly4DO5jdXtIASZ+uPZElsmsKg= -github.com/codeready-toolchain/toolchain-common v0.0.0-20250313203311-0bce6563576f h1:qBUZ/xBNgDMf5Y0tObJ85/9GcDSgtUf8WFfCrNSxaZ4= -github.com/codeready-toolchain/toolchain-common v0.0.0-20250313203311-0bce6563576f/go.mod h1:0dopWh1MWi9eg2d5RAd4uSUmXfDMqz4Yk2oNPHF15+Y= +github.com/codeready-toolchain/toolchain-common v0.0.0-20250430091615-95cf792ac171 h1:qK+3ECtkMiWT8IyPhwetajh04E/XRSzC2NN5PQnSDSM= +github.com/codeready-toolchain/toolchain-common v0.0.0-20250430091615-95cf792ac171/go.mod h1:Rd64iFrlcocihpLq4qIsCMkjN/1invHHaU1AMq19OWA= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= From cc76d93d787a389f3a4bcea98215d5feac824371 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Thu, 20 Mar 2025 20:25:07 +0100 Subject: [PATCH 2/4] define the field manager name to use with SSA --- cmd/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/main.go b/cmd/main.go index 8fcb2daf3..63a16b514 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -70,7 +70,8 @@ var ( ) const ( - memberClientTimeout = 3 * time.Second + memberClientTimeout = 3 * time.Second + hostOperatorFieldManager = "kubesaw-host-operator" ) func init() { From 6ad7e3fe5bc9b62c6540bd5f3a87cec3aac00c35 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Thu, 20 Mar 2025 20:25:07 +0100 Subject: [PATCH 3/4] use the SSA apply client for deploying the registration service --- cmd/main.go | 1 + .../toolchainconfig_controller.go | 50 ++++--------------- .../toolchainconfig_controller_test.go | 18 +++---- 3 files changed, 20 insertions(+), 49 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 63a16b514..65cdf1863 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -294,6 +294,7 @@ func main() { // nolint:gocyclo Client: mgr.GetClient(), GetMembersFunc: commoncluster.GetMemberClusters, Scheme: mgr.GetScheme(), + FieldManager: hostOperatorFieldManager, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ToolchainConfig") os.Exit(1) diff --git a/controllers/toolchainconfig/toolchainconfig_controller.go b/controllers/toolchainconfig/toolchainconfig_controller.go index 2ace242f4..dba872144 100644 --- a/controllers/toolchainconfig/toolchainconfig_controller.go +++ b/controllers/toolchainconfig/toolchainconfig_controller.go @@ -21,7 +21,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -36,7 +35,6 @@ var DefaultReconcile = reconcile.Result{RequeueAfter: 10 * time.Second} // SetupWithManager sets up the controller with the Manager. func (r *Reconciler) SetupWithManager(mgr manager.Manager) error { - regServiceTemplate, err := registrationservice.GetDeploymentTemplate() if err != nil { return errs.Wrap(err, "unable to decode the registration service deployment") @@ -58,6 +56,7 @@ type Reconciler struct { GetMembersFunc cluster.GetMemberClustersFunc Scheme *runtime.Scheme RegServiceTemplate *templatev1.Template + FieldManager string } //+kubebuilder:rbac:groups=toolchain.dev.openshift.com,resources=toolchainconfigs,verbs=get;list;watch;create;update;patch;delete @@ -101,7 +100,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. // Load the latest config and secrets into the cache cfg, err := ForceLoadToolchainConfig(r.Client) if err != nil { - return reconcile.Result{}, r.WrapErrorWithStatusUpdate(ctx, toolchainConfig, r.setStatusDeployRegistrationServiceFailed, err, "failed to load the latest configuration") + return reconcile.Result{}, r.WrapErrorWithStatusUpdate(ctx, toolchainConfig, r.setStatusDeployRegistrationServiceFailed, fmt.Errorf("failed to load the latest configuration: %w", err)) } // Deploy registration service @@ -128,34 +127,18 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. func (r *Reconciler) ensureRegistrationService(ctx context.Context, toolchainConfig *toolchainv1alpha1.ToolchainConfig, vars templateVars) error { // process template with variables taken from the RegistrationService CRD - cl := applycl.NewApplyClient(r.Client) + cl := applycl.NewSSAApplyClient(r.Client, r.FieldManager) toolchainObjects, err := template.NewProcessor(r.Scheme).Process(r.RegServiceTemplate.DeepCopy(), vars) if err != nil { - return r.WrapErrorWithStatusUpdate(ctx, toolchainConfig, r.setStatusDeployRegistrationServiceFailed, err, "failed to process registration service template") + return r.WrapErrorWithStatusUpdate(ctx, toolchainConfig, r.setStatusDeployRegistrationServiceFailed, fmt.Errorf("failed to process registration service template: %w", err)) } // create all objects that are within the template, and update only when the object has changed. - var updated []string - for _, toolchainObject := range toolchainObjects { - createdOrUpdated, err := cl.ApplyObject(ctx, toolchainObject, applycl.SetOwner(toolchainConfig)) - if err != nil { - return r.WrapErrorWithStatusUpdate(ctx, toolchainConfig, r.setStatusDeployRegistrationServiceFailed, err, "failed to apply registration service object %s", toolchainObject.GetName()) - } - if createdOrUpdated { - gvk, err := apiutil.GVKForObject(toolchainObject, r.Scheme) - if err != nil { - return r.WrapErrorWithStatusUpdate(ctx, toolchainConfig, r.setStatusDeployRegistrationServiceFailed, err, "failed to determine GroupVersionKind for registration service object %s", toolchainObject.GetName()) - } - updated = append(updated, fmt.Sprintf("%s: %s", gvk.Kind, toolchainObject.GetName())) - } + if err := cl.Apply(ctx, toolchainObjects, applycl.SetOwnerReference(toolchainConfig)); err != nil { + return r.WrapErrorWithStatusUpdate(ctx, toolchainConfig, r.setStatusDeployRegistrationServiceFailed, fmt.Errorf("failed to apply registration service: %w", err)) } logger := log.FromContext(ctx) - if len(updated) > 0 { - logger.Info("Updated registration service resources", "updated resources", updated) - return r.updateStatusCondition(ctx, toolchainConfig, ToRegServiceDeploying(fmt.Sprintf("updated resources: %s", updated)), false) - } - logger.Info("All objects in registration service template have been created and are up-to-date") return r.updateStatusCondition(ctx, toolchainConfig, ToRegServiceDeployComplete(), false) } @@ -200,19 +183,16 @@ func (r *Reconciler) WrapErrorWithStatusUpdate( ctx context.Context, toolchainConfig *toolchainv1alpha1.ToolchainConfig, statusUpdater func(ctx context.Context, toolchainConfig *toolchainv1alpha1.ToolchainConfig, message string) error, - providedError error, - format string, - args ...interface{}, + err error, ) error { - if providedError == nil { + if err == nil { return nil } - wrappedErr := errs.Wrapf(providedError, format, args...) - if err := statusUpdater(ctx, toolchainConfig, wrappedErr.Error()); err != nil { + if err := statusUpdater(ctx, toolchainConfig, err.Error()); err != nil { logger := log.FromContext(ctx) logger.Error(err, "status update failed") } - return wrappedErr + return err } // ToSyncComplete condition when the sync completed with success @@ -243,16 +223,6 @@ func ToRegServiceDeployComplete() toolchainv1alpha1.Condition { } } -// ToRegServiceDeploying condition when deploying is in progress -func ToRegServiceDeploying(msg string) toolchainv1alpha1.Condition { - return toolchainv1alpha1.Condition{ - Type: toolchainv1alpha1.ToolchainConfigRegServiceDeploy, - Status: corev1.ConditionFalse, - Reason: toolchainv1alpha1.ToolchainConfigRegServiceDeployingReason, - Message: msg, - } -} - // ToRegServiceDeployFailure condition when an error occurred func ToRegServiceDeployFailure(msg string) toolchainv1alpha1.Condition { return toolchainv1alpha1.Condition{ diff --git a/controllers/toolchainconfig/toolchainconfig_controller_test.go b/controllers/toolchainconfig/toolchainconfig_controller_test.go index 7d3e588a4..1fdec87d1 100644 --- a/controllers/toolchainconfig/toolchainconfig_controller_test.go +++ b/controllers/toolchainconfig/toolchainconfig_controller_test.go @@ -86,7 +86,7 @@ func TestReconcile(t *testing.T) { Exists(). HasConditions( toolchainconfig.ToSyncComplete(), - toolchainconfig.ToRegServiceDeploying("updated resources: [ServiceAccount: registration-service Role: registration-service RoleBinding: registration-service Deployment: registration-service Route: registration-service Service: registration-service Service: registration-service-metrics Route: api Service: api Service: proxy-metrics-service]")). + toolchainconfig.ToRegServiceDeployComplete()). HasNoSyncErrors() // check member1 config @@ -233,8 +233,8 @@ func TestReconcile(t *testing.T) { config := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.AutomaticApproval().Enabled(true)) hostCl := test.NewFakeClient(t, config) - hostCl.MockCreate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.CreateOption) error { - return fmt.Errorf("create error") + hostCl.MockPatch = func(ctx context.Context, obj runtimeclient.Object, patch runtimeclient.Patch, opts ...runtimeclient.PatchOption) error { + return fmt.Errorf("patch error") } members := NewGetMemberClusters(NewMemberClusterWithTenantRole(t, "member1", corev1.ConditionTrue), NewMemberClusterWithTenantRole(t, "member2", corev1.ConditionTrue)) controller := newController(t, hostCl, members) @@ -243,14 +243,14 @@ func TestReconcile(t *testing.T) { _, err := controller.Reconcile(context.TODO(), newRequest()) // then - require.EqualError(t, err, "failed to apply registration service object registration-service: unable to create resource of kind: ServiceAccount, version: v1: create error") + require.EqualError(t, err, "failed to apply registration service: unable to patch '/v1, Kind=ServiceAccount' called 'registration-service' in namespace 'toolchain-host-operator': patch error") actual, err := toolchainconfig.GetToolchainConfig(hostCl) require.NoError(t, err) assert.True(t, actual.AutomaticApproval().IsEnabled()) testconfig.AssertThatToolchainConfig(t, test.HostOperatorNs, hostCl). Exists(). HasConditions( - toolchainconfig.ToRegServiceDeployFailure("failed to apply registration service object registration-service: unable to create resource of kind: ServiceAccount, version: v1: create error")). + toolchainconfig.ToRegServiceDeployFailure("failed to apply registration service: unable to patch '/v1, Kind=ServiceAccount' called 'registration-service' in namespace 'toolchain-host-operator': patch error")). HasNoSyncErrors() }) @@ -274,7 +274,7 @@ func TestReconcile(t *testing.T) { Exists(). HasConditions( toolchainconfig.ToSyncFailure(), - toolchainconfig.ToRegServiceDeploying("updated resources: [ServiceAccount: registration-service Role: registration-service RoleBinding: registration-service Deployment: registration-service Route: registration-service Service: registration-service Service: registration-service-metrics Route: api Service: api Service: proxy-metrics-service]")). + toolchainconfig.ToRegServiceDeployComplete()). HasSyncErrors(map[string]string{"missing-member": "specific member configuration exists but no matching toolchaincluster was found"}) }) }) @@ -296,7 +296,7 @@ func TestWrapErrorWithUpdateStatus(t *testing.T) { } // test - err := controller.WrapErrorWithStatusUpdate(ctx, config, statusUpdater, nil, "failed to load the latest configuration") + err := controller.WrapErrorWithStatusUpdate(ctx, config, statusUpdater, nil) require.NoError(t, err) }) @@ -308,7 +308,7 @@ func TestWrapErrorWithUpdateStatus(t *testing.T) { } // test - err := controller.WrapErrorWithStatusUpdate(ctx, config, statusUpdater, fmt.Errorf("underlying error"), "failed to load the latest configuration") + err := controller.WrapErrorWithStatusUpdate(ctx, config, statusUpdater, fmt.Errorf("failed to load the latest configuration: %w", fmt.Errorf("underlying error"))) require.EqualError(t, err, "failed to load the latest configuration: underlying error") }) @@ -319,7 +319,7 @@ func TestWrapErrorWithUpdateStatus(t *testing.T) { } // when - err := controller.WrapErrorWithStatusUpdate(ctx, config, statusUpdater, fmt.Errorf("underlying error"), "failed to load the latest configuration") + err := controller.WrapErrorWithStatusUpdate(ctx, config, statusUpdater, fmt.Errorf("failed to load the latest configuration: %w", fmt.Errorf("underlying error"))) // then require.EqualError(t, err, "failed to load the latest configuration: underlying error") From d8c0e915d66a9ac9e29dd69890c069c9edfec687 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Fri, 2 May 2025 13:48:41 +0200 Subject: [PATCH 4/4] use a constant instead of a field for the field manager used by the host operator --- cmd/main.go | 6 +----- controllers/toolchainconfig/toolchainconfig_controller.go | 4 ++-- pkg/constants/field_manager.go | 5 +++++ 3 files changed, 8 insertions(+), 7 deletions(-) create mode 100644 pkg/constants/field_manager.go diff --git a/cmd/main.go b/cmd/main.go index 65cdf1863..d9eced7c4 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -69,10 +69,7 @@ var ( setupLog = ctrl.Log.WithName("setup") ) -const ( - memberClientTimeout = 3 * time.Second - hostOperatorFieldManager = "kubesaw-host-operator" -) +const memberClientTimeout = 3 * time.Second func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) @@ -294,7 +291,6 @@ func main() { // nolint:gocyclo Client: mgr.GetClient(), GetMembersFunc: commoncluster.GetMemberClusters, Scheme: mgr.GetScheme(), - FieldManager: hostOperatorFieldManager, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ToolchainConfig") os.Exit(1) diff --git a/controllers/toolchainconfig/toolchainconfig_controller.go b/controllers/toolchainconfig/toolchainconfig_controller.go index dba872144..4d813130b 100644 --- a/controllers/toolchainconfig/toolchainconfig_controller.go +++ b/controllers/toolchainconfig/toolchainconfig_controller.go @@ -7,6 +7,7 @@ import ( "time" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + "github.com/codeready-toolchain/host-operator/pkg/constants" "github.com/codeready-toolchain/host-operator/pkg/templates/registrationservice" applycl "github.com/codeready-toolchain/toolchain-common/pkg/client" "github.com/codeready-toolchain/toolchain-common/pkg/cluster" @@ -56,7 +57,6 @@ type Reconciler struct { GetMembersFunc cluster.GetMemberClustersFunc Scheme *runtime.Scheme RegServiceTemplate *templatev1.Template - FieldManager string } //+kubebuilder:rbac:groups=toolchain.dev.openshift.com,resources=toolchainconfigs,verbs=get;list;watch;create;update;patch;delete @@ -127,7 +127,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. func (r *Reconciler) ensureRegistrationService(ctx context.Context, toolchainConfig *toolchainv1alpha1.ToolchainConfig, vars templateVars) error { // process template with variables taken from the RegistrationService CRD - cl := applycl.NewSSAApplyClient(r.Client, r.FieldManager) + cl := applycl.NewSSAApplyClient(r.Client, constants.HostOperatorFieldManager) toolchainObjects, err := template.NewProcessor(r.Scheme).Process(r.RegServiceTemplate.DeepCopy(), vars) if err != nil { return r.WrapErrorWithStatusUpdate(ctx, toolchainConfig, r.setStatusDeployRegistrationServiceFailed, fmt.Errorf("failed to process registration service template: %w", err)) diff --git a/pkg/constants/field_manager.go b/pkg/constants/field_manager.go new file mode 100644 index 000000000..797bb2180 --- /dev/null +++ b/pkg/constants/field_manager.go @@ -0,0 +1,5 @@ +package constants + +// HostOperatorFieldManager is the field manager we want to use in the managed fields +// of objects deployed by the host operator. +const HostOperatorFieldManager = "kubesaw-host-operator"