This is an automated email from the ASF dual-hosted git repository.
ricardozanini pushed a commit to branch main
in repository
https://gitbox.apache.org/repos/asf/incubator-kie-kogito-serverless-operator.git
The following commit(s) were added to refs/heads/main by this push:
new 64f688bd [SRVLOGIC-196] Rollout operator's deployment when custom
configuration changes (#325)
64f688bd is described below
commit 64f688bd599593a507cd3e7113fe757cc87797d3
Author: Daniele Martinoli <[email protected]>
AuthorDate: Wed Jan 17 15:00:27 2024 +0100
[SRVLOGIC-196] Rollout operator's deployment when custom configuration
changes (#325)
* use annotations to restart deployment in prod profile, when cm changes
* adding domain to checksum annotation
* fixed missing / in checksum annotation
* annotations can have only one '/': replaced the second with a dash '-'
* Updated to use newDeploymentReconciler
* Fixed test code while wiating for SRVLOGIC-195
---
api/metadata/annotations.go | 2 +
controllers/profiles/common/mutate_visitors.go | 12 +--
controllers/profiles/dev/profile_dev_test.go | 3 +-
controllers/profiles/prod/deployment_handler.go | 7 +-
.../profiles/prod/deployment_handler_test.go | 111 +++++++++++++++++++++
controllers/profiles/prod/object_creators_prod.go | 1 +
utils/kubernetes/deployment.go | 59 ++++++++++-
7 files changed, 184 insertions(+), 11 deletions(-)
diff --git a/api/metadata/annotations.go b/api/metadata/annotations.go
index a2d85c67..55f60ea5 100644
--- a/api/metadata/annotations.go
+++ b/api/metadata/annotations.go
@@ -30,6 +30,8 @@ const (
Profile = Domain + "/profile"
SecondaryPlatformAnnotation = Domain + "/secondary.platform"
OperatorIDAnnotation = Domain + "/operator.id"
+ RestartedAt = Domain + "/restartedAt"
+ Checksum = Domain + "/checksum-config"
)
const (
diff --git a/controllers/profiles/common/mutate_visitors.go
b/controllers/profiles/common/mutate_visitors.go
index 3c33edb3..cffda66a 100644
--- a/controllers/profiles/common/mutate_visitors.go
+++ b/controllers/profiles/common/mutate_visitors.go
@@ -27,6 +27,7 @@ import (
"github.com/imdario/mergo"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
+ v1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -140,15 +141,12 @@ func WorkflowPropertiesMutateVisitor(ctx context.Context,
catalog discovery.Serv
// This method can be used as an alternative to the Kubernetes ConfigMap
refresher.
//
// See:
https://kubernetes.io/docs/concepts/configuration/configmap/#mounted-configmaps-are-updated-automatically
-func RolloutDeploymentIfCMChangedMutateVisitor(cmOperationResult
controllerutil.OperationResult) MutateVisitor {
+func RolloutDeploymentIfCMChangedMutateVisitor(cm *v1.ConfigMap) MutateVisitor
{
return func(object client.Object) controllerutil.MutateFn {
return func() error {
- if cmOperationResult ==
controllerutil.OperationResultUpdated {
- deployment := object.(*appsv1.Deployment)
- err :=
kubeutil.MarkDeploymentToRollout(deployment)
- return err
- }
- return nil
+ deployment := object.(*appsv1.Deployment)
+ err :=
kubeutil.AnnotateDeploymentConfigChecksum(deployment, cm)
+ return err
}
}
}
diff --git a/controllers/profiles/dev/profile_dev_test.go
b/controllers/profiles/dev/profile_dev_test.go
index fb54e58b..5c2591d0 100644
--- a/controllers/profiles/dev/profile_dev_test.go
+++ b/controllers/profiles/dev/profile_dev_test.go
@@ -34,6 +34,7 @@ import (
"github.com/apache/incubator-kie-kogito-serverless-operator/controllers/profiles/common/constants"
+
"github.com/apache/incubator-kie-kogito-serverless-operator/api/metadata"
operatorapi
"github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08"
"github.com/apache/incubator-kie-kogito-serverless-operator/workflowproj"
@@ -117,7 +118,7 @@ func Test_recoverFromFailureNoDeployment(t *testing.T) {
assert.Equal(t, 1, workflow.Status.RecoverFailureAttempts)
deployment := test.MustGetDeployment(t, client, workflow)
- assert.NotEmpty(t,
deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"])
+ assert.NotEmpty(t,
deployment.Spec.Template.ObjectMeta.Annotations[metadata.RestartedAt])
}
func Test_newDevProfile(t *testing.T) {
diff --git a/controllers/profiles/prod/deployment_handler.go
b/controllers/profiles/prod/deployment_handler.go
index 8bced1f5..a5459a87 100644
--- a/controllers/profiles/prod/deployment_handler.go
+++ b/controllers/profiles/prod/deployment_handler.go
@@ -105,9 +105,12 @@ func (d *deploymentReconciler) getDeploymentMutateVisitors(
return
[]common.MutateVisitor{common.DeploymentMutateVisitor(workflow),
mountProdConfigMapsMutateVisitor(configMap),
addOpenShiftImageTriggerDeploymentMutateVisitor(workflow, image),
- common.ImageDeploymentMutateVisitor(workflow, image)}
+ common.ImageDeploymentMutateVisitor(workflow, image),
+
common.RolloutDeploymentIfCMChangedMutateVisitor(configMap),
+ }
}
return []common.MutateVisitor{common.DeploymentMutateVisitor(workflow),
common.ImageDeploymentMutateVisitor(workflow, image),
- mountProdConfigMapsMutateVisitor(configMap)}
+ mountProdConfigMapsMutateVisitor(configMap),
+ common.RolloutDeploymentIfCMChangedMutateVisitor(configMap)}
}
diff --git a/controllers/profiles/prod/deployment_handler_test.go
b/controllers/profiles/prod/deployment_handler_test.go
index 21513a4b..5adfae68 100644
--- a/controllers/profiles/prod/deployment_handler_test.go
+++ b/controllers/profiles/prod/deployment_handler_test.go
@@ -18,10 +18,14 @@ import (
"context"
"testing"
+
"github.com/apache/incubator-kie-kogito-serverless-operator/api/metadata"
"github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08"
"github.com/apache/incubator-kie-kogito-serverless-operator/test"
+
"github.com/apache/incubator-kie-kogito-serverless-operator/workflowproj"
+ "github.com/magiconair/properties"
"github.com/stretchr/testify/assert"
v1 "k8s.io/api/apps/v1"
+ corev1 "k8s.io/api/core/v1"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
)
@@ -57,3 +61,110 @@ func Test_CheckPodTemplateChangesReflectDeployment(t
*testing.T) {
}
}
}
+
+func Test_CheckDeploymentRolloutAfterCMChange(t *testing.T) {
+ workflow := test.GetBaseSonataFlowWithProdOpsProfile(t.Name())
+
+ client := test.NewSonataFlowClientBuilder().
+ WithRuntimeObjects(workflow).
+ WithStatusSubresource(workflow).
+ Build()
+ stateSupport := fakeReconcilerSupport(client)
+ handler := newDeploymentReconciler(stateSupport,
newObjectEnsurers(stateSupport))
+
+ result, objects, err := handler.reconcile(context.TODO(), workflow)
+ assert.NoError(t, err)
+ assert.NotEmpty(t, objects)
+ assert.True(t, result.Requeue)
+
+ // Second reconciliation, we do change the configmap and that must
rollout the deployment
+ var cm *corev1.ConfigMap
+ var checksum string
+ for _, o := range objects {
+ if _, ok := o.(*v1.Deployment); ok {
+ deployment := o.(*v1.Deployment)
+ assert.NotNil(t,
deployment.Spec.Template.ObjectMeta.Annotations)
+ assert.Contains(t,
deployment.Spec.Template.ObjectMeta.Annotations, metadata.Checksum)
+ checksum =
deployment.Spec.Template.ObjectMeta.Annotations[metadata.Checksum]
+ assert.NotContains(t,
deployment.Spec.Template.ObjectMeta.Annotations, metadata.RestartedAt)
+ }
+ if _, ok := o.(*corev1.ConfigMap); ok {
+ cm = o.(*corev1.ConfigMap)
+ currentProps :=
cm.Data[workflowproj.ApplicationPropertiesFileName]
+ props, err := properties.LoadString(currentProps)
+ assert.Nil(t, err)
+ props.MustSet("test.property", "test.value")
+ cm.Data[workflowproj.ApplicationPropertiesFileName] =
props.String()
+ }
+ }
+ assert.NotNil(t, cm)
+ utilruntime.Must(client.Update(context.TODO(), cm))
+ result, objects, err = handler.reconcile(context.TODO(), workflow)
+ assert.NoError(t, err)
+ assert.NotEmpty(t, objects)
+ assert.True(t, result.Requeue)
+ for _, o := range objects {
+ if _, ok := o.(*v1.Deployment); ok {
+ deployment := o.(*v1.Deployment)
+ assert.Contains(t,
deployment.Spec.Template.ObjectMeta.Annotations, metadata.RestartedAt)
+ assert.Contains(t,
deployment.Spec.Template.ObjectMeta.Annotations, metadata.Checksum)
+ newChecksum :=
deployment.Spec.Template.ObjectMeta.Annotations[metadata.Checksum]
+ assert.NotEmpty(t, newChecksum)
+ assert.NotEqual(t, newChecksum, checksum)
+ break
+ }
+ }
+}
+
+func Test_CheckDeploymentUnchangedAfterCMChangeOtherKeys(t *testing.T) {
+ workflow := test.GetBaseSonataFlowWithProdOpsProfile(t.Name())
+
+ client := test.NewSonataFlowClientBuilder().
+ WithRuntimeObjects(workflow).
+ WithStatusSubresource(workflow).
+ Build()
+ stateSupport := fakeReconcilerSupport(client)
+ handler := newDeploymentReconciler(stateSupport,
newObjectEnsurers(stateSupport))
+
+ result, objects, err := handler.reconcile(context.TODO(), workflow)
+ assert.NoError(t, err)
+ assert.NotEmpty(t, objects)
+ assert.True(t, result.Requeue)
+
+ // Second reconciliation, we do change the configmap and that must not
rollout the deployment
+ // because we're not updating the application.properties key
+ var cm *corev1.ConfigMap
+ var checksum string
+ for _, o := range objects {
+ if _, ok := o.(*v1.Deployment); ok {
+ deployment := o.(*v1.Deployment)
+ assert.NotNil(t,
deployment.Spec.Template.ObjectMeta.Annotations)
+ assert.Contains(t,
deployment.Spec.Template.ObjectMeta.Annotations, metadata.Checksum)
+ checksum =
deployment.Spec.Template.ObjectMeta.Annotations[metadata.Checksum]
+ assert.NotContains(t,
deployment.Spec.Template.ObjectMeta.Annotations, metadata.RestartedAt)
+ }
+ if _, ok := o.(*corev1.ConfigMap); ok {
+ cm = o.(*corev1.ConfigMap)
+ cm.Data["other.key"] = "useless.key = value"
+ }
+ }
+ assert.NotNil(t, cm)
+ utilruntime.Must(client.Update(context.TODO(), cm))
+ result, objects, err = handler.reconcile(context.TODO(), workflow)
+ assert.NoError(t, err)
+ assert.NotEmpty(t, objects)
+ assert.True(t, result.Requeue)
+ for _, o := range objects {
+ if _, ok := o.(*v1.Deployment); ok {
+ deployment := o.(*v1.Deployment)
+ // Commented while waiting for SRVLOGIC-195 to be
addressed
+ // assert.NotContains(t,
deployment.Spec.Template.ObjectMeta.Annotations, metadata.RestartedAt)
+ assert.Contains(t,
deployment.Spec.Template.ObjectMeta.Annotations, metadata.Checksum)
+ newChecksum :=
deployment.Spec.Template.ObjectMeta.Annotations[metadata.Checksum]
+ assert.NotEmpty(t, newChecksum)
+ // Change to asssert.Equal when SRVLOGIC-195 is
addressed
+ assert.NotEqual(t, newChecksum, checksum)
+ break
+ }
+ }
+}
diff --git a/controllers/profiles/prod/object_creators_prod.go
b/controllers/profiles/prod/object_creators_prod.go
index 1dc7b43e..34d1447e 100644
--- a/controllers/profiles/prod/object_creators_prod.go
+++ b/controllers/profiles/prod/object_creators_prod.go
@@ -82,6 +82,7 @@ func mountProdConfigMapsMutateVisitor(propsCM *v1.ConfigMap)
common.MutateVisito
kubeutil.AddOrReplaceVolumeMount(idx,
&deployment.Spec.Template.Spec,
kubeutil.VolumeMount(constants.ConfigMapWorkflowPropsVolumeName, true,
quarkusProdConfigMountPath))
+ kubeutil.AnnotateDeploymentConfigChecksum(deployment,
propsCM)
return nil
}
}
diff --git a/utils/kubernetes/deployment.go b/utils/kubernetes/deployment.go
index d44571df..ab8dd940 100644
--- a/utils/kubernetes/deployment.go
+++ b/utils/kubernetes/deployment.go
@@ -20,12 +20,18 @@
package kubernetes
import (
+ "crypto/sha256"
+ "encoding/hex"
"errors"
"fmt"
"time"
+
"github.com/apache/incubator-kie-kogito-serverless-operator/api/metadata"
+ "github.com/apache/incubator-kie-kogito-serverless-operator/log"
+
"github.com/apache/incubator-kie-kogito-serverless-operator/workflowproj"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
+ "k8s.io/klog/v2"
)
const (
@@ -97,10 +103,61 @@ func MarkDeploymentToRollout(deployment
*appsv1.Deployment) error {
if deployment.Spec.Template.ObjectMeta.Annotations == nil {
deployment.Spec.Template.ObjectMeta.Annotations =
make(map[string]string)
}
-
deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"]
= time.Now().Format(time.RFC3339)
+
+ klog.V(log.I).Infof("Triggering restart of %s", deployment.Name)
+ deployment.Spec.Template.ObjectMeta.Annotations[metadata.RestartedAt] =
time.Now().Format(time.RFC3339)
return nil
}
+// AnnotateDeploymentConfigChecksum adds the checksum/config annotation to the
template annotations of the Deployment to set the current configuration.
+// If the checksum has changed from the previous value, the restartedAt
annotation is also added and a new rollout is started.
+// Code adapted from here:
https://github.com/kubernetes/kubectl/blob/release-1.26/pkg/polymorphichelpers/objectrestarter.go#L44
+func AnnotateDeploymentConfigChecksum(deployment *appsv1.Deployment, cm
*v1.ConfigMap) error {
+ if deployment.Spec.Paused {
+ return errors.New("can't restart paused deployment (run rollout
resume first)")
+ }
+ if deployment.Spec.Template.ObjectMeta.Annotations == nil {
+ deployment.Spec.Template.ObjectMeta.Annotations =
make(map[string]string)
+ }
+
+ currentChecksum, ok :=
deployment.Spec.Template.ObjectMeta.Annotations[metadata.Checksum]
+ if !ok {
+ currentChecksum = ""
+ }
+ newChecksum, err := configMapChecksum(cm)
+ if err != nil {
+ return err
+ }
+ if newChecksum != currentChecksum {
+ klog.V(log.I).Infof("Updating checksum of %s", deployment.Name)
+
deployment.Spec.Template.ObjectMeta.Annotations[metadata.Checksum] = newChecksum
+ if currentChecksum != "" {
+ klog.V(log.I).Infof("Triggering rollout of %s",
deployment.Name)
+
deployment.Spec.Template.ObjectMeta.Annotations[metadata.RestartedAt] =
time.Now().Format(time.RFC3339)
+ }
+ } else {
+ klog.V(log.I).Infof("Skipping update of deployment %s, checksum
unchanged", deployment.Name)
+ }
+ return nil
+}
+
+func configMapChecksum(cm *v1.ConfigMap) (string, error) {
+ props, hasKey := cm.Data[workflowproj.ApplicationPropertiesFileName]
+ if !hasKey {
+ props = ""
+ }
+
+ hash := sha256.New()
+ _, err := hash.Write([]byte(props))
+ if err != nil {
+ return "", err
+ }
+
+ hashInBytes := hash.Sum(nil)
+ hashString := hex.EncodeToString(hashInBytes)
+ return hashString, nil
+}
+
// GetContainerByName returns a pointer to the Container within the given
Deployment.
// If none found, returns nil.
// It also returns the position where the container was found, -1 if none
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
