Skip to content

Commit ecda27e

Browse files
ezekiel-alexrodezekiel-alexrod
committed
scripts: address review round 5
- Hardcode Jinja build_image_name() directly in Makefile patches, remove __IMAGE__ placeholder and image_placeholder config field - Add 'delete' config field to remove scaffold files (replaces hardcoded .devcontainer removal and controller_test patches) - Simplify controller patches: keep scaffold kubebuilder marker format, remove Copyright changes and commented-out code - Improve raw_copy directory handling: diff and skip if identical, error with diff if different (idempotent re-runs) - Remove auto-pin of YAML config (never modify the config file) - Remove _update_yaml() function (no longer needed) - Fix f-string formatting per review suggestions - Sync BUMPING.md: remove stale image_placeholder references
1 parent 432380b commit ecda27e

File tree

11 files changed

+89
-413
lines changed

11 files changed

+89
-413
lines changed

‎BUMPING.md‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ Each operator has a config directory at `scripts/upgrade-operator-sdk/<name>/` c
188188
- **Versions**: `operator_sdk_version`, `go_toolchain` (optional pin), `k8s_libs` (optional pin)
189189
- **Scaffold**: `repo`, `domain`, `apis` (with `group`, `version`, `kind`, `namespaced`). The operator name is derived from the config directory name.
190190
- **Raw copy**: `raw_copy` -- directories or files copied as-is from backup (purely custom code with no scaffold equivalent: `pkg/`, `version/`, `config/metalk8s/`, `salt/`, individual test/helper files)
191-
- **Post-processing**: `image_placeholder`, `extra_commands`
191+
- **Post-processing**: `extra_commands`
192192

193193
### Patch files
194194

@@ -205,10 +205,9 @@ apply cleanly, look for `.rej` files and resolve manually.
205205

206206
Patch files use `__PLACEHOLDER__` tokens for runtime values:
207207

208-
| Placeholder | Replaced with | Source |
209-
| ----------------- | ------------------------------- | ---------- |
210-
| `__GOTOOLCHAIN__` | Detected/pinned Go toolchain | `Makefile` |
211-
| `__IMAGE__` | `image_placeholder` from config | `Makefile` |
208+
| Placeholder | Replaced with | Source |
209+
| ----------------- | ---------------------------- | ---------- |
210+
| `__GOTOOLCHAIN__` | Detected/pinned Go toolchain | `Makefile` |
212211

213212
New `.patch` files in the patches directory are automatically picked up.
214213

‎tools/upgrade-operator-sdk/operator/config.yaml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,12 @@ raw_copy:
2727
- api/v1alpha1/virtualippool_types_test.go
2828
- api/v1alpha1/v1alpha1_suite_test.go
2929

30-
image_placeholder: '{{ build_image_name("metalk8s-operator") }}'
30+
# Files/dirs to delete from the scaffold (not needed or incompatible).
31+
delete:
32+
- .devcontainer
33+
- .github
34+
- internal/controller/clusterconfig_controller_test.go
35+
- internal/controller/virtualippool_controller_test.go
3136

3237
extra_commands:
3338
- ["make", "metalk8s"]

‎tools/upgrade-operator-sdk/operator/patches/Makefile.patch‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@
1212
+metalk8s: manifests kustomize ## Generate MetalK8s resulting manifests
1313
+ mkdir -p deploy
1414
+ $(KUSTOMIZE) build config/metalk8s | \
15-
+ sed 's/BUILD_IMAGE_CLUSTER_OPERATOR:latest/__IMAGE__/' > deploy/manifests.yaml
15+
+ sed 's/BUILD_IMAGE_CLUSTER_OPERATOR:latest/{{ build_image_name("metalk8s-operator") }}/' > deploy/manifests.yaml

‎tools/upgrade-operator-sdk/operator/patches/clusterconfig_controller.patch‎

Lines changed: 14 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,5 @@
11
--- a/internal/controller/clusterconfig_controller.go
22
+++ b/internal/controller/clusterconfig_controller.go
3-
@@ -1,5 +1,5 @@
4-
/*
5-
-Copyright 2026.
6-
+Copyright 2022.
7-
8-
Licensed under the Apache License, Version 2.0 (the "License");
9-
you may not use this file except in compliance with the License.
103
@@ -17,14 +17,10 @@
114
package controller
125

@@ -23,27 +16,18 @@
2316
)
2417

2518
// ClusterConfigReconciler reconciles a ClusterConfig object
26-
@@ -33,9 +29,13 @@
27-
Scheme *runtime.Scheme
28-
}
29-
30-
-// +kubebuilder:rbac:groups=metalk8s.scality.com,resources=clusterconfigs,verbs=get;list;watch;create;update;patch;delete
31-
-// +kubebuilder:rbac:groups=metalk8s.scality.com,resources=clusterconfigs/status,verbs=get;update;patch
32-
-// +kubebuilder:rbac:groups=metalk8s.scality.com,resources=clusterconfigs/finalizers,verbs=update
33-
+//+kubebuilder:rbac:groups=metalk8s.scality.com,resources=clusterconfigs,verbs=get;list;watch;create;update;patch;delete
34-
+//+kubebuilder:rbac:groups=metalk8s.scality.com,resources=clusterconfigs/status,verbs=get;update;patch
35-
+//+kubebuilder:rbac:groups=metalk8s.scality.com,resources=clusterconfigs/finalizers,verbs=update
36-
+
37-
+//+kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch;create;update;patch;delete
38-
+//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch
39-
+//+kubebuilder:rbac:groups=metalk8s.scality.com,resources=virtualippools,verbs=get;list;watch;create;update;patch;delete
19+
@@ -37,27 +33,11 @@
20+
// +kubebuilder:rbac:groups=metalk8s.scality.com,resources=clusterconfigs/status,verbs=get;update;patch
21+
// +kubebuilder:rbac:groups=metalk8s.scality.com,resources=clusterconfigs/finalizers,verbs=update
4022

41-
// Reconcile is part of the main kubernetes reconciliation loop which aims to
42-
// move the current state of the cluster closer to the desired state.
43-
@@ -45,19 +45,18 @@
44-
// the user.
45-
//
46-
// For more details, check Reconcile and its Result here:
23+
-// Reconcile is part of the main kubernetes reconciliation loop which aims to
24+
-// move the current state of the cluster closer to the desired state.
25+
-// TODO(user): Modify the Reconcile function to compare the state specified by
26+
-// the ClusterConfig object against the actual cluster state, and then
27+
-// perform operations to make the cluster state reflect the state specified by
28+
-// the user.
29+
-//
30+
-// For more details, check Reconcile and its Result here:
4731
-// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.21.0/pkg/reconcile
4832
-func (r *ClusterConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
4933
- _ = logf.FromContext(ctx)
@@ -52,12 +36,9 @@
5236
-
5337
- return ctrl.Result{}, nil
5438
-}
55-
+// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.12.1/pkg/reconcile
56-
+//func (r *ClusterConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
57-
+// _ = log.FromContext(ctx)
58-
+//
59-
+// return ctrl.Result{}, nil
60-
+//}
39+
+// +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch;create;update;patch;delete
40+
+// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
41+
+// +kubebuilder:rbac:groups=metalk8s.scality.com,resources=virtualippools,verbs=get;list;watch;create;update;patch;delete
6142

6243
// SetupWithManager sets up the controller with the Manager.
6344
func (r *ClusterConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
@@ -66,8 +47,4 @@
6647
- Named("clusterconfig").
6748
- Complete(r)
6849
+ return clusterconfig.Add(mgr)
69-
+
70-
+ //return ctrl.NewControllerManagedBy(mgr).
71-
+ // For(&metalk8sscalitycomv1alpha1.ClusterConfig{}).
72-
+ // Complete(r)
7350
}

‎tools/upgrade-operator-sdk/operator/patches/clusterconfig_controller_test.patch‎

Lines changed: 0 additions & 87 deletions
This file was deleted.

‎tools/upgrade-operator-sdk/operator/patches/virtualippool_controller.patch‎

Lines changed: 16 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,5 @@
11
--- a/internal/controller/virtualippool_controller.go
22
+++ b/internal/controller/virtualippool_controller.go
3-
@@ -1,5 +1,5 @@
4-
/*
5-
-Copyright 2026.
6-
+Copyright 2022.
7-
8-
Licensed under the Apache License, Version 2.0 (the "License");
9-
you may not use this file except in compliance with the License.
103
@@ -17,14 +17,10 @@
114
package controller
125

@@ -23,29 +16,23 @@
2316
)
2417

2518
// VirtualIPPoolReconciler reconciles a VirtualIPPool object
26-
@@ -33,9 +29,15 @@
27-
Scheme *runtime.Scheme
28-
}
19+
@@ -37,27 +33,12 @@
20+
// +kubebuilder:rbac:groups=metalk8s.scality.com,resources=virtualippools/status,verbs=get;update;patch
21+
// +kubebuilder:rbac:groups=metalk8s.scality.com,resources=virtualippools/finalizers,verbs=update
2922

30-
-// +kubebuilder:rbac:groups=metalk8s.scality.com,resources=virtualippools,verbs=get;list;watch;create;update;patch;delete
31-
-// +kubebuilder:rbac:groups=metalk8s.scality.com,resources=virtualippools/status,verbs=get;update;patch
32-
-// +kubebuilder:rbac:groups=metalk8s.scality.com,resources=virtualippools/finalizers,verbs=update
33-
+//+kubebuilder:rbac:groups=metalk8s.scality.com,resources=virtualippools,verbs=get;list;watch;create;update;patch;delete
34-
+//+kubebuilder:rbac:groups=metalk8s.scality.com,resources=virtualippools/status,verbs=get;update;patch
35-
+//+kubebuilder:rbac:groups=metalk8s.scality.com,resources=virtualippools/finalizers,verbs=update
23+
+// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
24+
+// +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch
25+
+// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;create;update;patch;delete
26+
+// +kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=get;list;watch;create;update;patch;delete
3627
+
37-
+//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch
38-
+
39-
+//+kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch
40-
+//+kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;create;update;patch;delete
41-
+//+kubebuilder:rbac:groups="apps",resources=daemonsets,verbs=get;list;watch;create;update;patch;delete
42-
43-
// Reconcile is part of the main kubernetes reconciliation loop which aims to
44-
// move the current state of the cluster closer to the desired state.
45-
@@ -45,19 +47,18 @@
46-
// the user.
47-
//
48-
// For more details, check Reconcile and its Result here:
28+
-// Reconcile is part of the main kubernetes reconciliation loop which aims to
29+
-// move the current state of the cluster closer to the desired state.
30+
-// TODO(user): Modify the Reconcile function to compare the state specified by
31+
-// the VirtualIPPool object against the actual cluster state, and then
32+
-// perform operations to make the cluster state reflect the state specified by
33+
-// the user.
34+
-//
35+
-// For more details, check Reconcile and its Result here:
4936
-// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.21.0/pkg/reconcile
5037
-func (r *VirtualIPPoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
5138
- _ = logf.FromContext(ctx)
@@ -54,22 +41,12 @@
5441
-
5542
- return ctrl.Result{}, nil
5643
-}
57-
+// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.12.2/pkg/reconcile
58-
+//func (r *VirtualIPPoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
59-
+// _ = log.FromContext(ctx)
60-
+//
61-
+// return ctrl.Result{}, nil
62-
+//}
63-
44+
-
6445
// SetupWithManager sets up the controller with the Manager.
6546
func (r *VirtualIPPoolReconciler) SetupWithManager(mgr ctrl.Manager) error {
6647
- return ctrl.NewControllerManagedBy(mgr).
6748
- For(&metalk8sscalitycomv1alpha1.VirtualIPPool{}).
6849
- Named("virtualippool").
6950
- Complete(r)
7051
+ return virtualippool.Add(mgr)
71-
+
72-
+ //return ctrl.NewControllerManagedBy(mgr).
73-
+ // For(&metalk8sscalitycomv1alpha1.VirtualIPPool{}).
74-
+ // Complete(r)
7552
}

‎tools/upgrade-operator-sdk/operator/patches/virtualippool_controller_test.patch‎

Lines changed: 0 additions & 87 deletions
This file was deleted.

‎tools/upgrade-operator-sdk/storage-operator/config.yaml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,13 @@ raw_copy:
2020
- salt
2121
- api/v1alpha1/volume_types_test.go
2222
- internal/controller/slice.go
23+
- internal/controller/volume_controller_test.go
2324

24-
image_placeholder: '{{ build_image_name("storage-operator") }}'
25+
# Files/dirs to delete from the scaffold (not needed or incompatible).
26+
delete:
27+
- .devcontainer
28+
- .github
29+
- internal/controller/volume_controller_test.go
2530

2631
extra_commands:
2732
- ["make", "metalk8s"]

‎tools/upgrade-operator-sdk/storage-operator/patches/Makefile.patch‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@
1212
+metalk8s: manifests kustomize ## Generate MetalK8s resulting manifests
1313
+ mkdir -p deploy
1414
+ $(KUSTOMIZE) build config/metalk8s | \
15-
+ sed 's/BUILD_IMAGE_CLUSTER_OPERATOR:latest/__IMAGE__/' > deploy/manifests.yaml
15+
+ sed 's/BUILD_IMAGE_CLUSTER_OPERATOR:latest/{{ build_image_name("storage-operator") }}/' > deploy/manifests.yaml

0 commit comments

Comments
 (0)