Skip to content

Commit d1d1487

Browse files
kstrenkovakstrenkova
committed
Allow CR updates
This PR introduces new test-operator functionality that will allow users to update their CRs and simultaneously restart the the pod. Currently, any change to the CR will not apply in the pod. The only way a user can use updated CR is to remove the pod and create a new one.
1 parent 42f48d1 commit d1d1487

11 files changed

Lines changed: 114 additions & 20 deletions

File tree

‎api/v1beta1/ansibletest_webhook.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ limitations under the License.
2323
package v1beta1
2424

2525
import (
26+
"errors"
27+
2628
"k8s.io/apimachinery/pkg/runtime"
2729
"k8s.io/apimachinery/pkg/util/validation/field"
2830
logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -74,8 +76,14 @@ func (r *AnsibleTest) ValidateCreate() (admission.Warnings, error) {
7476
func (r *AnsibleTest) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
7577
ansibletestlog.Info("validate update", "name", r.Name)
7678

77-
// TODO(user): fill in your validation logic upon object update.
78-
return nil, nil
79+
oldAnsibleTest, ok := old.(*AnsibleTest)
80+
if !ok || oldAnsibleTest == nil {
81+
return nil, errors.New("unable to convert existing object")
82+
}
83+
84+
allWarnings := admission.Warnings{}
85+
allWarnings = CheckSpecUpdated(allWarnings, oldAnsibleTest.Spec, r.Spec, r.Kind)
86+
return allWarnings, nil
7987
}
8088

8189
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type

‎api/v1beta1/common_webhook.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"reflect"
66

7+
"github.com/google/go-cmp/cmp"
78
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
89
apierrors "k8s.io/apimachinery/pkg/api/errors"
910
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -75,6 +76,9 @@ const (
7576
"set to true. Please, consider setting %[1]s.Spec.SELinuxLevel. This " +
7677
"ensures that the copying of the logs to the PV is completed without any " +
7778
"complications."
79+
80+
// WarnSpecUpdated
81+
WarnSpecUpdated = "%s CR updated. The pod will be recreated to apply changes."
7882
)
7983

8084
const (
@@ -193,3 +197,11 @@ func BuildValidationError(kind, name string, errs field.ErrorList) error {
193197
}
194198
return nil
195199
}
200+
201+
// CheckSpecUpdated returns warning if spec has changed
202+
func CheckSpecUpdated(allWarn admission.Warnings, oldSpec, newSpec interface{}, kind string) admission.Warnings {
203+
if !cmp.Equal(oldSpec, newSpec) {
204+
allWarn = append(allWarn, fmt.Sprintf(WarnSpecUpdated, kind))
205+
}
206+
return allWarn
207+
}

‎api/v1beta1/horizontest_webhook.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ limitations under the License.
2323
package v1beta1
2424

2525
import (
26+
"errors"
27+
2628
"k8s.io/apimachinery/pkg/runtime"
2729
logf "sigs.k8s.io/controller-runtime/pkg/log"
2830
"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -59,8 +61,14 @@ func (r *HorizonTest) ValidateCreate() (admission.Warnings, error) {
5961
func (r *HorizonTest) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
6062
horizontestlog.Info("validate update", "name", r.Name)
6163

62-
// TODO(user): fill in your validation logic upon object update.
63-
return nil, nil
64+
oldHorizonTest, ok := old.(*HorizonTest)
65+
if !ok || oldHorizonTest == nil {
66+
return nil, errors.New("unable to convert existing object")
67+
}
68+
69+
allWarnings := admission.Warnings{}
70+
allWarnings = CheckSpecUpdated(allWarnings, oldHorizonTest.Spec, r.Spec, r.Kind)
71+
return allWarnings, nil
6472
}
6573

6674
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type

‎api/v1beta1/tempest_webhook.go‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"errors"
2727
"fmt"
2828

29-
"github.com/google/go-cmp/cmp"
3029
"k8s.io/apimachinery/pkg/runtime"
3130
"k8s.io/apimachinery/pkg/util/validation/field"
3231
logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -108,14 +107,9 @@ func (r *Tempest) ValidateUpdate(old runtime.Object) (admission.Warnings, error)
108107
return nil, errors.New("unable to convert existing object")
109108
}
110109

111-
if !cmp.Equal(oldTempest.Spec, r.Spec) {
112-
warnings := admission.Warnings{}
113-
warnings = append(warnings, "You are updating an already existing instance of a "+
114-
"Tempest CR! Be aware that changes won't be applied.")
115-
116-
return warnings, errors.New("updating an existing Tempest CR is not supported")
117-
}
118-
return nil, nil
110+
allWarnings := admission.Warnings{}
111+
allWarnings = CheckSpecUpdated(allWarnings, oldTempest.Spec, r.Spec, r.Kind)
112+
return allWarnings, nil
119113
}
120114

121115
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type

‎api/v1beta1/tobiko_webhook.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ limitations under the License.
2323
package v1beta1
2424

2525
import (
26+
"errors"
2627
"fmt"
2728

2829
"k8s.io/apimachinery/pkg/runtime"
@@ -82,8 +83,14 @@ func (r *Tobiko) ValidateCreate() (admission.Warnings, error) {
8283
func (r *Tobiko) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
8384
tobikolog.Info("validate update", "name", r.Name)
8485

85-
// TODO(user): fill in your validation logic upon object update.
86-
return nil, nil
86+
oldTobiko, ok := old.(*Tobiko)
87+
if !ok || oldTobiko == nil {
88+
return nil, errors.New("unable to convert existing object")
89+
}
90+
91+
allWarnings := admission.Warnings{}
92+
allWarnings = CheckSpecUpdated(allWarnings, oldTobiko.Spec, r.Spec, r.Kind)
93+
return allWarnings, nil
8794
}
8895

8996
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type

‎internal/ansibletest/pod.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
func Pod(
1313
instance *testv1beta1.AnsibleTest,
1414
labels map[string]string,
15+
annotations map[string]string,
1516
podName string,
1617
logsPVCName string,
1718
mountCerts bool,
@@ -20,7 +21,7 @@ func Pod(
2021
containerImage string,
2122
) *corev1.Pod {
2223
return util.BuildTestPod(
23-
nil, // No annotations
24+
annotations,
2425
PodCapabilities,
2526
containerImage,
2627
instance.Name,

‎internal/controller/ansibletest_controller.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
9696
func (r *AnsibleTestReconciler) buildAnsibleTestPod(
9797
ctx context.Context,
9898
instance *testv1beta1.AnsibleTest,
99-
labels, _ map[string]string,
99+
labels, annotations map[string]string,
100100
workflowStepIndex int,
101101
pvcIndex int,
102102
) (*corev1.Pod, error) {
@@ -114,6 +114,7 @@ func (r *AnsibleTestReconciler) buildAnsibleTestPod(
114114
return ansibletest.Pod(
115115
instance,
116116
labels,
117+
annotations,
117118
podName,
118119
logsPVCName,
119120
mountCerts,

‎internal/controller/common.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package controller
22

33
import (
44
"context"
5+
"encoding/json"
56
"errors"
67
"fmt"
78
"reflect"
@@ -913,3 +914,56 @@ func MergeSections(main interface{}, workflow interface{}) {
913914
}
914915
}
915916
}
917+
918+
// CalculateConfigHash calculates a hash of the entire Spec to detect any changes
919+
func CalculateConfigHash(instance client.Object) string {
920+
v := reflect.ValueOf(instance)
921+
spec, err := SafetyCheck(v, "Spec")
922+
if err != nil {
923+
return ""
924+
}
925+
926+
data, err := json.Marshal(spec.Interface())
927+
if err != nil {
928+
return ""
929+
}
930+
931+
hash := sha256.Sum256(data)
932+
return fmt.Sprintf("%x", hash[:8])
933+
}
934+
935+
// CheckConfigChange checks if the spec has changed and recreates the pod if needed
936+
func (r *Reconciler) CheckConfigChange(
937+
ctx context.Context,
938+
instance client.Object,
939+
workflowStepIndex int,
940+
) (ctrl.Result, error) {
941+
Log := r.GetLogger()
942+
943+
newHash := CalculateConfigHash(instance)
944+
if newHash == "" {
945+
return ctrl.Result{}, nil
946+
}
947+
948+
podName := r.GetPodName(instance, workflowStepIndex)
949+
pod, err := r.GetPod(ctx, podName, instance.GetNamespace())
950+
if err != nil {
951+
return ctrl.Result{}, nil
952+
}
953+
954+
currentHash := pod.Annotations["test.openstack.org/config-hash"]
955+
if currentHash == "" || currentHash == newHash {
956+
return ctrl.Result{}, nil
957+
}
958+
959+
Log.Info("Configuration changed, deleting pod for recreation",
960+
"pod", pod.Name,
961+
"currentHash", currentHash,
962+
"newHash", newHash)
963+
964+
if err := r.Client.Delete(ctx, pod); err != nil && !k8s_errors.IsNotFound(err) {
965+
return ctrl.Result{}, err
966+
}
967+
968+
return ctrl.Result{Requeue: true}, nil
969+
}

‎internal/controller/common_controller.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,11 @@ func CommonReconcile[T TestResource](
188188
MergeSections(spec, workflowStepData)
189189
}
190190

191+
// Check for config changes and handle pod recreation
192+
if ctrlResult, err := r.CheckConfigChange(ctx, instance, workflowStepIndex); err != nil || (ctrlResult != ctrl.Result{}) {
193+
return ctrlResult, err
194+
}
195+
191196
parallel := false
192197
if config.GetParallel != nil {
193198
parallel = config.GetParallel(instance)
@@ -285,6 +290,9 @@ func CommonReconcile[T TestResource](
285290
return ctrlResult, nil
286291
}
287292

293+
serviceAnnotations := make(map[string]string)
294+
serviceAnnotations["test.openstack.org/config-hash"] = CalculateConfigHash(instance)
295+
288296
// Generate ConfigMaps containing test configuration
289297
if config.NeedsConfigMaps {
290298
err = config.GenerateServiceConfigMaps(ctx, helper, serviceLabels, instance, workflowStepIndex)
@@ -300,7 +308,6 @@ func CommonReconcile[T TestResource](
300308
conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage)
301309
}
302310

303-
var serviceAnnotations map[string]string
304311
if config.NeedsNetworkAttachments {
305312
annotations, ctrlResult, err := r.EnsureNetworkAttachments(
306313
ctx,

‎internal/controller/horizontest_controller.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func (r *HorizonTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
9696
func (r *HorizonTestReconciler) buildHorizonTestPod(
9797
ctx context.Context,
9898
instance *testv1beta1.HorizonTest,
99-
labels, _ map[string]string,
99+
labels, annotations map[string]string,
100100
workflowStepIndex int,
101101
pvcIndex int,
102102
) (*corev1.Pod, error) {
@@ -115,6 +115,7 @@ func (r *HorizonTestReconciler) buildHorizonTestPod(
115115
return horizontest.Pod(
116116
instance,
117117
labels,
118+
annotations,
118119
podName,
119120
logsPVCName,
120121
mountCerts,

0 commit comments

Comments
 (0)