From 760f8788f03f972f31825248e95bdc72f94545db Mon Sep 17 00:00:00 2001 From: An Tran Date: Thu, 20 Nov 2025 16:13:34 +1000 Subject: [PATCH 1/2] Trigger system-app hook when image changes --- pkg/3scale/amp/operator/system_reconciler.go | 82 ++++++++++++++++---- 1 file changed, 67 insertions(+), 15 deletions(-) diff --git a/pkg/3scale/amp/operator/system_reconciler.go b/pkg/3scale/amp/operator/system_reconciler.go index 2f6699e9b..f9fddc484 100644 --- a/pkg/3scale/amp/operator/system_reconciler.go +++ b/pkg/3scale/amp/operator/system_reconciler.go @@ -15,6 +15,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" k8sappsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -156,30 +157,38 @@ func (r *SystemReconciler) Reconcile() (reconcile.Result, error) { return reconcile.Result{}, err } - // If the system-app Deployment revision has changed, delete the PreHook/PostHook Jobs so they can be recreated - revisionChanged, err := helper.HasAppRevisionChanged(component.SystemAppPreHookJobName, currentAppDeploymentRevision, currentNameSpace, r.Client()) + // Check if the system image has changed (indicating an upgrade) + imageChanged, err := hasSystemImageChanged(currentNameSpace, ampImages.Options.SystemImage, r.Client()) if err != nil { return reconcile.Result{}, err } - if revisionChanged { - err = helper.DeleteJob(r.Context(), component.SystemAppPreHookJobName, currentNameSpace, r.Client()) - if err != nil { - return reconcile.Result{}, err - } - } - revisionChanged, err = helper.HasAppRevisionChanged(component.SystemAppPostHookJobName, currentAppDeploymentRevision, currentNameSpace, r.Client()) + + // If the system-app Deployment revision has changed, delete the PreHook/PostHook Jobs so they can be recreated + revisionChanged, err := helper.HasAppRevisionChanged(component.SystemAppPreHookJobName, currentAppDeploymentRevision, currentNameSpace, r.Client()) if err != nil { return reconcile.Result{}, err } - if revisionChanged { - err = helper.DeleteJob(r.Context(), component.SystemAppPostHookJobName, currentNameSpace, r.Client()) - if err != nil { - return reconcile.Result{}, err + + // SystemApp PreHook Job + var preHookJob *v1.Job + if imageChanged { + if !revisionChanged && imageChanged { + err = helper.DeleteJob(r.Context(), component.SystemAppPreHookJobName, currentNameSpace, r.Client()) + if err != nil { + return reconcile.Result{}, err + } + } + preHookJob = system.AppPreHookJob(ampImages.Options.SystemImage, currentNameSpace, currentAppDeploymentRevision+1) + } else { + if revisionChanged { + err = helper.DeleteJob(r.Context(), component.SystemAppPreHookJobName, currentNameSpace, r.Client()) + if err != nil { + return reconcile.Result{}, err + } } + preHookJob = system.AppPreHookJob(ampImages.Options.SystemImage, currentNameSpace, currentAppDeploymentRevision) } - // SystemApp PreHook Job - preHookJob := system.AppPreHookJob(ampImages.Options.SystemImage, currentNameSpace, currentAppDeploymentRevision) err = r.ReconcileJob(preHookJob, reconcilers.CreateOnlyMutator) if err != nil { return reconcile.Result{}, err @@ -238,6 +247,18 @@ func (r *SystemReconciler) Reconcile() (reconcile.Result, error) { systemComponentsReady = false } + revisionChanged, err = helper.HasAppRevisionChanged(component.SystemAppPostHookJobName, currentAppDeploymentRevision, currentNameSpace, r.Client()) + if err != nil { + return reconcile.Result{}, err + } + + if revisionChanged { + err = helper.DeleteJob(r.Context(), component.SystemAppPostHookJobName, currentNameSpace, r.Client()) + if err != nil { + return reconcile.Result{}, err + } + } + // SystemApp PostHook Job if systemComponentsReady { err = r.ReconcileJob(system.AppPostHookJob(ampImages.Options.SystemImage, currentNameSpace, currentAppDeploymentRevision), reconcilers.CreateOnlyMutator) @@ -374,6 +395,37 @@ func getSystemAppDeploymentRevision(namespace string, client k8sclient.Client) ( return strconv.ParseInt(v, 10, 64) } +// hasSystemImageChanged checks if the system image has changed compared to the currently deployed image +// This indicates an upgrade scenario where hooks should be run +func hasSystemImageChanged(namespace string, desiredImage string, client k8sclient.Client) (bool, error) { + deployment := &k8sappsv1.Deployment{} + err := client.Get(context.TODO(), k8sclient.ObjectKey{ + Namespace: namespace, + Name: component.SystemAppDeploymentName, + }, deployment) + + // Return error if can't get Deployment + if err != nil && !k8serr.IsNotFound(err) { + return false, fmt.Errorf("error getting deployment %s: %w", component.SystemAppDeploymentName, err) + } + + // If the Deployment doesn't exist yet, this is a fresh install, not an upgrade + if k8serr.IsNotFound(err) { + return false, nil + } + + // Check if the system-app container image has changed + for _, container := range deployment.Spec.Template.Spec.Containers { + if container.Name == "system-master" || container.Name == "system-provider" || container.Name == "system-developer" { + if container.Image != desiredImage { + return true, nil + } + } + } + + return false, nil +} + func (r *SystemReconciler) systemAppDeploymentResourceMutator(desired, existing *k8sappsv1.Deployment) (bool, error) { desiredName := helper.ObjectInfo(desired) update := false From 615e1731da0254490ec3c07c5ee437b7448f4b82 Mon Sep 17 00:00:00 2001 From: An Tran Date: Thu, 20 Nov 2025 20:00:18 +1000 Subject: [PATCH 2/2] Addressing PR feedback --- pkg/3scale/amp/operator/system_reconciler.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/3scale/amp/operator/system_reconciler.go b/pkg/3scale/amp/operator/system_reconciler.go index f9fddc484..5275148c1 100644 --- a/pkg/3scale/amp/operator/system_reconciler.go +++ b/pkg/3scale/amp/operator/system_reconciler.go @@ -172,14 +172,20 @@ func (r *SystemReconciler) Reconcile() (reconcile.Result, error) { // SystemApp PreHook Job var preHookJob *v1.Job if imageChanged { - if !revisionChanged && imageChanged { + // Version upgrades, or image changes, occur before the Deployment is + // updated, so the revision version remains unchanged. + // Delete the old job so we can trigger a new one + if !revisionChanged { err = helper.DeleteJob(r.Context(), component.SystemAppPreHookJobName, currentNameSpace, r.Client()) if err != nil { return reconcile.Result{}, err } } + // Increase revision version by 1 to avoid rerun the job once the Deployment + // is updated with the new image preHookJob = system.AppPreHookJob(ampImages.Options.SystemImage, currentNameSpace, currentAppDeploymentRevision+1) } else { + // normal rollout if revisionChanged { err = helper.DeleteJob(r.Context(), component.SystemAppPreHookJobName, currentNameSpace, r.Client()) if err != nil {