Skip to content

Commit e8f96e9

Browse files
scripts: address review round 4 -- operator_dir as CLI arg, raw_copy, new patches
- Move operator_dir from YAML config to --operator-dir CLI argument - Remove REPO_ROOT and __file__-based path deduction (KISS) - Rename backup_paths to raw_copy, reduce to purely custom dirs (pkg/, version/, config/metalk8s/, salt/) - Generate patch files for CRD types and controllers (previously raw-copied from backup, now scaffold + patch) - Remove Dockerfile modification (scaffold FROM golang is kept as-is) - Fail on missing raw_copy paths instead of warning - Use Path.is_dir() instead of endswith("/") convention - Remove zz_generated filtering (no longer needed) - Neutralize scaffold-generated controller tests via patches (incompatible with our delegation pattern, never used) - Add --forward flag to patch command to avoid false "reversed patch"
1 parent 3db697e commit e8f96e9

13 files changed

+2394
-112
lines changed

BUMPING.md

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,8 @@ Target versions are pinned in `scripts/upgrade-operator-sdk/<name>/config.yaml`:
141141

142142
```yaml
143143
operator_sdk_version: v1.42.1 # target operator-sdk release
144-
go_toolchain: go1.25.8 # pin Go toolchain (for GOTOOLCHAIN)
145-
k8s_libs: v0.33.9 # pin k8s.io libs version
144+
go_toolchain: go1.24.13 # pin Go toolchain (for GOTOOLCHAIN)
145+
k8s_libs: v0.33.10 # pin k8s.io libs version
146146
```
147147
148148
After scaffolding, the script detects the latest available versions (operator-sdk
@@ -159,22 +159,24 @@ This is CI-friendly: zero interactive input during reconciliation.
159159
160160
### Running the upgrade
161161
162-
The script processes one operator at a time. Run it once per operator:
162+
The script processes one operator at a time:
163163
164164
```bash
165-
python3 scripts/upgrade-operator-sdk/upgrade.py operator
166-
python3 scripts/upgrade-operator-sdk/upgrade.py storage-operator
167-
```
165+
python3 scripts/upgrade-operator-sdk/upgrade.py \
166+
--operator-dir operator \
167+
scripts/upgrade-operator-sdk/operator
168168

169-
The argument is the name of the config directory next to the script
170-
(i.e. `scripts/upgrade-operator-sdk/<name>/`). A full path can also be
171-
given for configs stored elsewhere.
169+
python3 scripts/upgrade-operator-sdk/upgrade.py \
170+
--operator-dir storage-operator \
171+
scripts/upgrade-operator-sdk/storage-operator
172+
```
172173

173174
Options:
174175

175176
```
177+
--operator-dir Path to the operator project directory (required)
176178
--skip-backup Reuse an existing .bak directory (no new backup)
177-
--clean-tools Delete .tmp/bin/ after the upgrade
179+
--clean-tools Remove tool cache after upgrade
178180
--yes, -y Skip the confirmation prompt
179181
```
180182

@@ -185,17 +187,23 @@ Each operator has a config directory at `scripts/upgrade-operator-sdk/<name>/` c
185187

186188
- **Versions**: `operator_sdk_version`, `go_toolchain` (optional pin), `k8s_libs` (optional pin)
187189
- **Scaffold**: `repo`, `domain`, `apis` (with `group`, `version`, `kind`, `namespaced`). The operator name is derived from the config directory name.
188-
- **Paths**: `operator_dir`, `backup_paths`
190+
- **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)
189191
- **Post-processing**: `image_placeholder`, `extra_commands`
190192

191193
### Patch files
192194

193-
MetalK8s-specific customizations to scaffold-generated files (`Dockerfile`, `Makefile`)
194-
are stored as GNU unified diff files in the `patches/` subdirectory next to `config.yaml`. The script
195-
applies them with `patch -p1` after scaffolding. If a patch does not apply cleanly,
196-
look for `.rej` files and resolve manually.
195+
All customizations to scaffold-generated files are stored as GNU unified diff
196+
files in the `patches/` subdirectory. This includes:
197+
198+
- **Dockerfile** and **Makefile** customizations
199+
- **CRD type definitions** (`*_types.go`)
200+
- **Controller implementations** (`*_controller.go`)
201+
- **Scaffold test stubs** (`*_controller_test.go`) -- neutralized when incompatible with the delegation pattern
202+
203+
The script applies them with `patch -p1` after scaffolding. If a patch does not
204+
apply cleanly, look for `.rej` files and resolve manually.
197205

198-
Patch files use `__PLACEHOLDER__` tokens for values from the YAML config:
206+
Patch files use `__PLACEHOLDER__` tokens for runtime values:
199207

200208
| Placeholder | Replaced with | Source |
201209
| ----------------- | ------------------------------- | ---------- |

scripts/upgrade-operator-sdk/operator/config.yaml

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
repo: github.com/scality/metalk8s/operator
22
domain: metalk8s.scality.com
3-
operator_dir: operator
43

54
operator_sdk_version: v1.42.1
65

7-
# Optional: pin versions. If absent or empty, the script detects the
8-
# latest patch from the scaffold's go.mod and offers to update this file.
6+
# Optional: pin versions. If absent, the script detects the latest
7+
# patch from the scaffold's go.mod and auto-pins them here.
98
go_toolchain: go1.24.13
109
k8s_libs: v0.33.10
1110

@@ -17,13 +16,16 @@ apis:
1716
kind: VirtualIPPool
1817
namespaced: true
1918

20-
backup_paths:
21-
- pkg/
22-
- version/
23-
- config/metalk8s/
24-
- api/
25-
- hack/
26-
- internal/controller/
19+
# Directories/files copied as-is from backup (purely custom, no scaffold equivalent).
20+
raw_copy:
21+
- pkg
22+
- version
23+
- config/metalk8s
24+
- api/v1alpha1/conditions.go
25+
- api/v1alpha1/conditions_test.go
26+
- api/v1alpha1/clusterconfig_types_test.go
27+
- api/v1alpha1/virtualippool_types_test.go
28+
- api/v1alpha1/v1alpha1_suite_test.go
2729

2830
image_placeholder: '{{ build_image_name("metalk8s-operator") }}'
2931

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
--- a/internal/controller/clusterconfig_controller.go
2+
+++ 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.
10+
@@ -17,14 +17,10 @@
11+
package controller
12+
13+
import (
14+
- "context"
15+
-
16+
+ "github.com/scality/metalk8s/operator/pkg/controller/clusterconfig"
17+
"k8s.io/apimachinery/pkg/runtime"
18+
ctrl "sigs.k8s.io/controller-runtime"
19+
"sigs.k8s.io/controller-runtime/pkg/client"
20+
- logf "sigs.k8s.io/controller-runtime/pkg/log"
21+
-
22+
- metalk8sscalitycomv1alpha1 "github.com/scality/metalk8s/operator/api/v1alpha1"
23+
)
24+
25+
// 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
40+
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:
47+
-// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.21.0/pkg/reconcile
48+
-func (r *ClusterConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
49+
- _ = logf.FromContext(ctx)
50+
-
51+
- // TODO(user): your logic here
52+
-
53+
- return ctrl.Result{}, nil
54+
-}
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+
+//}
61+
62+
// SetupWithManager sets up the controller with the Manager.
63+
func (r *ClusterConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
64+
- return ctrl.NewControllerManagedBy(mgr).
65+
- For(&metalk8sscalitycomv1alpha1.ClusterConfig{}).
66+
- Named("clusterconfig").
67+
- Complete(r)
68+
+ return clusterconfig.Add(mgr)
69+
+
70+
+ //return ctrl.NewControllerManagedBy(mgr).
71+
+ // For(&metalk8sscalitycomv1alpha1.ClusterConfig{}).
72+
+ // Complete(r)
73+
}
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
--- a/internal/controller/clusterconfig_controller_test.go
2+
+++ b/internal/controller/clusterconfig_controller_test.go
3+
@@ -1,84 +1 @@
4+
-/*
5+
-Copyright 2026.
6+
-
7+
-Licensed under the Apache License, Version 2.0 (the "License");
8+
-you may not use this file except in compliance with the License.
9+
-You may obtain a copy of the License at
10+
-
11+
- http://www.apache.org/licenses/LICENSE-2.0
12+
-
13+
-Unless required by applicable law or agreed to in writing, software
14+
-distributed under the License is distributed on an "AS IS" BASIS,
15+
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
-See the License for the specific language governing permissions and
17+
-limitations under the License.
18+
-*/
19+
-
20+
package controller
21+
-
22+
-import (
23+
- "context"
24+
-
25+
- . "github.com/onsi/ginkgo/v2"
26+
- . "github.com/onsi/gomega"
27+
- "k8s.io/apimachinery/pkg/api/errors"
28+
- "k8s.io/apimachinery/pkg/types"
29+
- "sigs.k8s.io/controller-runtime/pkg/reconcile"
30+
-
31+
- metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
32+
-
33+
- metalk8sscalitycomv1alpha1 "github.com/scality/metalk8s/operator/api/v1alpha1"
34+
-)
35+
-
36+
-var _ = Describe("ClusterConfig Controller", func() {
37+
- Context("When reconciling a resource", func() {
38+
- const resourceName = "test-resource"
39+
-
40+
- ctx := context.Background()
41+
-
42+
- typeNamespacedName := types.NamespacedName{
43+
- Name: resourceName,
44+
- Namespace: "default", // TODO(user):Modify as needed
45+
- }
46+
- clusterconfig := &metalk8sscalitycomv1alpha1.ClusterConfig{}
47+
-
48+
- BeforeEach(func() {
49+
- By("creating the custom resource for the Kind ClusterConfig")
50+
- err := k8sClient.Get(ctx, typeNamespacedName, clusterconfig)
51+
- if err != nil && errors.IsNotFound(err) {
52+
- resource := &metalk8sscalitycomv1alpha1.ClusterConfig{
53+
- ObjectMeta: metav1.ObjectMeta{
54+
- Name: resourceName,
55+
- Namespace: "default",
56+
- },
57+
- // TODO(user): Specify other spec details if needed.
58+
- }
59+
- Expect(k8sClient.Create(ctx, resource)).To(Succeed())
60+
- }
61+
- })
62+
-
63+
- AfterEach(func() {
64+
- // TODO(user): Cleanup logic after each test, like removing the resource instance.
65+
- resource := &metalk8sscalitycomv1alpha1.ClusterConfig{}
66+
- err := k8sClient.Get(ctx, typeNamespacedName, resource)
67+
- Expect(err).NotTo(HaveOccurred())
68+
-
69+
- By("Cleanup the specific resource instance ClusterConfig")
70+
- Expect(k8sClient.Delete(ctx, resource)).To(Succeed())
71+
- })
72+
- It("should successfully reconcile the resource", func() {
73+
- By("Reconciling the created resource")
74+
- controllerReconciler := &ClusterConfigReconciler{
75+
- Client: k8sClient,
76+
- Scheme: k8sClient.Scheme(),
77+
- }
78+
-
79+
- _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{
80+
- NamespacedName: typeNamespacedName,
81+
- })
82+
- Expect(err).NotTo(HaveOccurred())
83+
- // TODO(user): Add more specific assertions depending on your controller's reconciliation logic.
84+
- // Example: If you expect a certain status condition after reconciliation, verify it here.
85+
- })
86+
- })
87+
-})

0 commit comments

Comments
 (0)