Skip to content

Commit 640fe36

Browse files
fwieselfwiesel
committed
Demo for #300 
1 parent b9f45d1 commit 640fe36

1 file changed

Lines changed: 174 additions & 0 deletions

File tree

‎internal/controller/ssa_status_patch_test.go‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
/*
2+
SPDX-FileCopyrightText: Copyright 2025 SAP SE or an SAP affiliate company and cobaltcore-dev contributors
3+
SPDX-License-Identifier: Apache-2.0
4+
5+
Licensed under the Apache License, Version 2.0 (the "License");
6+
you may not use this file except in compliance with the License.
7+
You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
package controller
19+
20+
import (
21+
. "github.com/onsi/ginkgo/v2"
22+
. "github.com/onsi/gomega"
23+
apierrors "k8s.io/apimachinery/pkg/api/errors"
24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25+
k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1"
26+
k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
27+
28+
kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
29+
apiv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1"
30+
)
31+
32+
// ssaConditionCfg builds a HypervisorApplyConfiguration containing exactly one status condition.
33+
// Using apply configurations (all-pointer structs) ensures that only the explicitly set fields
34+
// appear in the apply body — critical for SSA to claim only those fields and leave all others
35+
// untouched. A plain kvmv1.Hypervisor struct cannot be used because non-pointer struct fields
36+
// (e.g. Status.OperatingSystem) are serialised even when zero-valued, causing the apply to claim
37+
// ownership of those fields and fail CRD validation.
38+
func ssaConditionCfg(name, condType string, condStatus metav1.ConditionStatus) *apiv1.HypervisorApplyConfiguration {
39+
return apiv1.Hypervisor(name, "").
40+
WithStatus(apiv1.HypervisorStatus().
41+
WithConditions(
42+
k8sacmetav1.Condition().
43+
WithType(condType).
44+
WithStatus(condStatus).
45+
WithReason("SpikeTest").
46+
WithMessage("set by " + condType).
47+
WithLastTransitionTime(metav1.Now()),
48+
),
49+
)
50+
}
51+
52+
// Spike test demonstrating that Server-Side Apply with +listType=map / +listMapKey=type on
53+
// the Conditions field gives each field manager independent per-condition ownership.
54+
// Without the x-kubernetes-list-type: map annotation in the CRD schema, the API server would
55+
// treat the whole conditions array as atomic — one manager would own all conditions and a second
56+
// manager applying a different condition would silently overwrite the first manager's condition.
57+
var _ = Describe("SSA per-condition field ownership (spike)", func() {
58+
const (
59+
hvName = "hv-ssa-spike"
60+
managerA = "controller-a"
61+
managerB = "controller-b"
62+
conditionA = "ConditionA"
63+
conditionB = "ConditionB"
64+
)
65+
66+
BeforeEach(func(ctx SpecContext) {
67+
hv := &kvmv1.Hypervisor{
68+
ObjectMeta: metav1.ObjectMeta{
69+
Name: hvName,
70+
},
71+
Spec: kvmv1.HypervisorSpec{
72+
LifecycleEnabled: true,
73+
},
74+
}
75+
Expect(k8sClient.Create(ctx, hv)).To(Succeed())
76+
DeferCleanup(func(ctx SpecContext) {
77+
Expect(k8sClient.Delete(ctx, hv)).To(Succeed())
78+
})
79+
})
80+
81+
It("allows two managers to independently own different conditions without overwriting each other", func(ctx SpecContext) {
82+
// --- Step 1: manager-a applies ConditionA=True ---
83+
By("manager-a applies ConditionA=True")
84+
Expect(k8sClient.Status().Apply(ctx,
85+
ssaConditionCfg(hvName, conditionA, metav1.ConditionTrue),
86+
k8sclient.FieldOwner(managerA),
87+
k8sclient.ForceOwnership,
88+
)).To(Succeed())
89+
90+
// --- Step 2: manager-b applies ConditionB=True ---
91+
By("manager-b applies ConditionB=True")
92+
Expect(k8sClient.Status().Apply(ctx,
93+
ssaConditionCfg(hvName, conditionB, metav1.ConditionTrue),
94+
k8sclient.FieldOwner(managerB),
95+
k8sclient.ForceOwnership,
96+
)).To(Succeed())
97+
98+
// --- Step 3: both conditions must coexist ---
99+
By("both conditions coexist on the same object")
100+
hv := &kvmv1.Hypervisor{}
101+
Expect(k8sClient.Get(ctx, k8sclient.ObjectKey{Name: hvName}, hv)).To(Succeed())
102+
103+
Expect(hv.Status.Conditions).To(ContainElement(SatisfyAll(
104+
HaveField("Type", conditionA),
105+
HaveField("Status", metav1.ConditionTrue),
106+
)), "ConditionA should still be present after manager-b applied ConditionB")
107+
Expect(hv.Status.Conditions).To(ContainElement(SatisfyAll(
108+
HaveField("Type", conditionB),
109+
HaveField("Status", metav1.ConditionTrue),
110+
)), "ConditionB should be present after manager-b applied it")
111+
112+
// --- Step 4: managed fields record per-condition ownership ---
113+
By("managed fields record per-condition ownership for each manager")
114+
managedFields := hv.GetManagedFields()
115+
116+
var entryA, entryB *metav1.ManagedFieldsEntry
117+
for i := range managedFields {
118+
e := &managedFields[i]
119+
switch {
120+
case e.Manager == managerA &&
121+
e.Operation == metav1.ManagedFieldsOperationApply &&
122+
e.Subresource == "status":
123+
entryA = e
124+
case e.Manager == managerB &&
125+
e.Operation == metav1.ManagedFieldsOperationApply &&
126+
e.Subresource == "status":
127+
entryB = e
128+
}
129+
}
130+
131+
Expect(entryA).NotTo(BeNil(), "expected a managed-fields entry for %s on the status subresource", managerA)
132+
Expect(entryB).NotTo(BeNil(), "expected a managed-fields entry for %s on the status subresource", managerB)
133+
134+
// The FieldsV1 JSON encodes owned list-map entries as k:{"type":"<condType>"},
135+
// so each manager's entry should contain only its own condition type value.
136+
Expect(string(entryA.FieldsV1.Raw)).To(ContainSubstring(conditionA),
137+
"manager-a's FieldsV1 should contain the key for ConditionA")
138+
Expect(string(entryA.FieldsV1.Raw)).NotTo(ContainSubstring(conditionB),
139+
"manager-a's FieldsV1 should NOT contain the key for ConditionB")
140+
Expect(string(entryB.FieldsV1.Raw)).To(ContainSubstring(conditionB),
141+
"manager-b's FieldsV1 should contain the key for ConditionB")
142+
Expect(string(entryB.FieldsV1.Raw)).NotTo(ContainSubstring(conditionA),
143+
"manager-b's FieldsV1 should NOT contain the key for ConditionA")
144+
145+
// --- Step 5: manager-a updates ConditionA; ConditionB must be untouched ---
146+
By("manager-a updates ConditionA to False; ConditionB must remain True")
147+
Expect(k8sClient.Status().Apply(ctx,
148+
ssaConditionCfg(hvName, conditionA, metav1.ConditionFalse),
149+
k8sclient.FieldOwner(managerA),
150+
k8sclient.ForceOwnership,
151+
)).To(Succeed())
152+
153+
Expect(k8sClient.Get(ctx, k8sclient.ObjectKey{Name: hvName}, hv)).To(Succeed())
154+
Expect(hv.Status.Conditions).To(ContainElement(SatisfyAll(
155+
HaveField("Type", conditionA),
156+
HaveField("Status", metav1.ConditionFalse),
157+
)), "ConditionA should now be False")
158+
Expect(hv.Status.Conditions).To(ContainElement(SatisfyAll(
159+
HaveField("Type", conditionB),
160+
HaveField("Status", metav1.ConditionTrue),
161+
)), "ConditionB must be untouched by manager-a's update")
162+
163+
// --- Step 6: manager-b cannot take over ConditionA without ForceOwnership ---
164+
By("manager-b cannot apply ConditionA without ForceOwnership — expects a 409 Conflict")
165+
err := k8sClient.Status().Apply(ctx,
166+
ssaConditionCfg(hvName, conditionA, metav1.ConditionTrue),
167+
k8sclient.FieldOwner(managerB),
168+
// intentionally no ForceOwnership
169+
)
170+
Expect(err).To(HaveOccurred())
171+
Expect(apierrors.IsConflict(err)).To(BeTrue(),
172+
"expected a 409 Conflict when taking over another manager's condition without ForceOwnership, got: %v", err)
173+
})
174+
})

0 commit comments

Comments
 (0)