Skip to content

Commit 83fae14

Browse files
Paused condition for MachineSets
This change sets the paused condition false when the MachineSet status has the AuthoritativeAPI set to MachineAPI, otherwise it sets it to true and stops reconciliation.
1 parent 684dba3 commit 83fae14

File tree

10 files changed

+245
-14
lines changed

10 files changed

+245
-14
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ require (
2929
k8s.io/kubectl v0.30.0
3030
k8s.io/utils v0.0.0-20240310230437-4693a0247e57
3131
sigs.k8s.io/cluster-api v1.6.1
32-
sigs.k8s.io/controller-runtime v0.18.3
32+
sigs.k8s.io/controller-runtime v0.18.4
3333
sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20240522175850-2e9781e9fc60
3434
sigs.k8s.io/yaml v1.4.0
3535
)

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,8 @@ sigs.k8s.io/cluster-api v1.6.1 h1:I34p/fwgRlEhs+o9cUhKXDwNNfPS3no0yJsd2bJyQVc=
780780
sigs.k8s.io/cluster-api v1.6.1/go.mod h1:DaxwruDvSaEYq5q6FREDaGzX6UsAVUCA99Sp8vfMHyQ=
781781
sigs.k8s.io/controller-runtime v0.18.3 h1:B5Wmmo8WMWK7izei+2LlXLVDGzMwAHBNLX68lwtlSR4=
782782
sigs.k8s.io/controller-runtime v0.18.3/go.mod h1:TVoGrfdpbA9VRFaRnKgk9P5/atA0pMwq+f+msb9M8Sg=
783+
sigs.k8s.io/controller-runtime v0.18.4 h1:87+guW1zhvuPLh1PHybKdYFLU0YJp4FhJRmiHvm5BZw=
784+
sigs.k8s.io/controller-runtime v0.18.4/go.mod h1:TVoGrfdpbA9VRFaRnKgk9P5/atA0pMwq+f+msb9M8Sg=
783785
sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20240522175850-2e9781e9fc60 h1:ihaeBTCFuEYPL1T1/FqAavDY7z5UcKSnWpnb+I3DYeM=
784786
sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20240522175850-2e9781e9fc60/go.mod h1:4+4tM2Es0ycqPedATtzPer5RTrUq3Xab59BYogt0mCE=
785787
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=

pkg/controller/machineset/controller.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@ import (
2525
"sync"
2626
"time"
2727

28+
openshiftfeatures "github.com/openshift/api/features"
2829
machinev1 "github.com/openshift/api/machine/v1beta1"
2930
"github.com/openshift/machine-api-operator/pkg/util"
31+
"github.com/openshift/machine-api-operator/pkg/util/conditions"
3032
corev1 "k8s.io/api/core/v1"
3133
apierrors "k8s.io/apimachinery/pkg/api/errors"
3234
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -45,6 +47,14 @@ import (
4547
"sigs.k8s.io/controller-runtime/pkg/source"
4648
)
4749

50+
const (
51+
PausedCondition machinev1.ConditionType = "Paused"
52+
53+
PausedConditionReason = "AuthoritativeAPI is not set to MachineAPI"
54+
55+
NotPausedConditionReason = "AuthoritativeAPI is set to MachineAPI"
56+
)
57+
4858
var (
4959
controllerKind = machinev1.SchemeGroupVersion.WithKind("MachineSet")
5060

@@ -170,6 +180,42 @@ func (r *ReconcileMachineSet) Reconcile(ctx context.Context, request reconcile.R
170180
return reconcile.Result{}, nil
171181
}
172182

183+
if r.gate.Enabled(featuregate.Feature(openshiftfeatures.FeatureGateMachineAPIMigration)) {
184+
machineSetCopy := machineSet.DeepCopy()
185+
// Check Status.AuthoritativeAPI. If it's not set to MachineAPI. Set the
186+
// paused condition true and return early.
187+
//
188+
// Once we have a webhook, we want to remove the check that the AuthoritativeAPI
189+
// field is populated.
190+
if machineSet.Status.AuthoritativeAPI != "" &&
191+
machineSet.Status.AuthoritativeAPI != machinev1.MachineAuthorityMachineAPI {
192+
conditions.Set(machineSetCopy, conditions.TrueConditionWithReason(
193+
PausedCondition,
194+
PausedConditionReason,
195+
"The AuthoritativeAPI is set to %s", machineSet.Status.AuthoritativeAPI,
196+
))
197+
198+
_, err := updateMachineSetStatus(r.Client, machineSet, machineSetCopy.Status)
199+
if err != nil {
200+
klog.Errorf("%v: error updating status: %v", machineSet.Name, err)
201+
}
202+
203+
klog.Infof("%v: machine is paused, taking no further action", machineSet.Name)
204+
return reconcile.Result{}, nil
205+
}
206+
207+
conditions.Set(machineSetCopy, conditions.FalseCondition(
208+
PausedCondition,
209+
NotPausedConditionReason,
210+
"The AuthoritativeAPI is set to %s", string(machineSet.Status.AuthoritativeAPI),
211+
))
212+
_, err := updateMachineSetStatus(r.Client, machineSet, machineSetCopy.Status)
213+
if err != nil {
214+
klog.Errorf("%v: error updating status: %v", machineSet.Name, err)
215+
}
216+
klog.Infof("%v: setting paused to false and continuing reconcile", machineSet.Name)
217+
}
218+
173219
result, err := r.reconcile(ctx, machineSet)
174220
if err != nil {
175221
klog.Errorf("Failed to reconcile MachineSet %q: %v", request.NamespacedName, err)

pkg/controller/machineset/controller_test.go

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ func TestMachineSetToMachines(t *testing.T) {
5757
},
5858
},
5959
},
60+
Status: machinev1.MachineSetStatus{
61+
AuthoritativeAPI: machinev1.MachineAuthorityMachineAPI,
62+
},
6063
},
6164
},
6265
}
@@ -153,7 +156,11 @@ func TestShouldExcludeMachine(t *testing.T) {
153156
expected bool
154157
}{
155158
{
156-
machineSet: machinev1.MachineSet{},
159+
machineSet: machinev1.MachineSet{
160+
Status: machinev1.MachineSetStatus{
161+
AuthoritativeAPI: machinev1.MachineAuthorityMachineAPI,
162+
},
163+
},
157164
machine: machinev1.Machine{
158165
ObjectMeta: metav1.ObjectMeta{
159166
Name: "withNoMatchingOwnerRef",
@@ -178,6 +185,9 @@ func TestShouldExcludeMachine(t *testing.T) {
178185
},
179186
},
180187
},
188+
Status: machinev1.MachineSetStatus{
189+
AuthoritativeAPI: machinev1.MachineAuthorityMachineAPI,
190+
},
181191
},
182192
machine: machinev1.Machine{
183193
ObjectMeta: metav1.ObjectMeta{
@@ -191,7 +201,11 @@ func TestShouldExcludeMachine(t *testing.T) {
191201
expected: false,
192202
},
193203
{
194-
machineSet: machinev1.MachineSet{},
204+
machineSet: machinev1.MachineSet{
205+
Status: machinev1.MachineSetStatus{
206+
AuthoritativeAPI: machinev1.MachineAuthorityMachineAPI,
207+
},
208+
},
195209
machine: machinev1.Machine{
196210
ObjectMeta: metav1.ObjectMeta{
197211
Name: "withDeletionTimestamp",
@@ -225,6 +239,9 @@ func TestAdoptOrphan(t *testing.T) {
225239
ObjectMeta: metav1.ObjectMeta{
226240
Name: "adoptOrphanMachine",
227241
},
242+
Status: machinev1.MachineSetStatus{
243+
AuthoritativeAPI: machinev1.MachineAuthorityMachineAPI,
244+
},
228245
}
229246
controller := true
230247
blockOwnerDeletion := true
@@ -306,7 +323,11 @@ var _ = Describe("MachineSet Reconcile", func() {
306323
},
307324
Spec: machinev1.MachineSetSpec{
308325
Template: machinev1.MachineTemplateSpec{},
309-
}}
326+
},
327+
Status: machinev1.MachineSetStatus{
328+
AuthoritativeAPI: machinev1.MachineAuthorityMachineAPI,
329+
},
330+
}
310331

311332
r.Client = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(ms).WithStatusSubresource(&machinev1.MachineSet{}).Build()
312333
})
@@ -335,6 +356,9 @@ var _ = Describe("MachineSet Reconcile", func() {
335356
Spec: machinev1.MachineSetSpec{
336357
Replicas: &replicas,
337358
},
359+
Status: machinev1.MachineSetStatus{
360+
AuthoritativeAPI: machinev1.MachineAuthorityMachineAPI,
361+
},
338362
}
339363

340364
ms.Spec.Selector.MatchLabels = map[string]string{

pkg/controller/machineset/machineset_controller_suite_test.go

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,17 @@ package machineset
1919
import (
2020
"context"
2121
"flag"
22+
"log"
23+
"os"
2224
"path/filepath"
2325
"testing"
2426
"time"
2527

2628
. "github.com/onsi/ginkgo/v2"
2729
. "github.com/onsi/gomega"
30+
configv1 "github.com/openshift/api/config/v1"
2831
machinev1 "github.com/openshift/api/machine/v1beta1"
32+
2933
"k8s.io/client-go/kubernetes/scheme"
3034
"k8s.io/client-go/rest"
3135
"k8s.io/klog/v2/textlogger"
@@ -38,9 +42,6 @@ func init() {
3842
textLoggerConfig := textlogger.NewConfig()
3943
textLoggerConfig.AddFlags(flag.CommandLine)
4044
logf.SetLogger(textlogger.NewLogger(textLoggerConfig))
41-
42-
// Register required object kinds with global scheme.
43-
_ = machinev1.Install(scheme.Scheme)
4445
}
4546

4647
const (
@@ -62,18 +63,38 @@ func TestMachinesetController(t *testing.T) {
6263

6364
var _ = BeforeSuite(func(ctx SpecContext) {
6465
By("bootstrapping test environment")
66+
6567
testEnv = &envtest.Environment{
66-
CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "install")},
68+
69+
CRDInstallOptions: envtest.CRDInstallOptions{
70+
Paths: []string{
71+
filepath.Join("..", "..", "..", "vendor", "github.com", "openshift", "api", "machine", "v1beta1", "zz_generated.crd-manifests", "0000_10_machine-api_01_machines-CustomNoUpgrade.crd.yaml"),
72+
filepath.Join("..", "..", "..", "vendor", "github.com", "openshift", "api", "machine", "v1beta1", "zz_generated.crd-manifests", "0000_10_machine-api_01_machinesets-CustomNoUpgrade.crd.yaml"),
73+
filepath.Join("..", "..", "..", "vendor", "github.com", "openshift", "api", "config", "v1", "zz_generated.crd-manifests", "0000_00_cluster-version-operator_01_clusterversions-CustomNoUpgrade.crd.yaml"),
74+
filepath.Join("..", "..", "..", "vendor", "github.com", "openshift", "api", "config", "v1", "zz_generated.crd-manifests", "0000_10_config-operator_01_featuregates.crd.yaml"),
75+
},
76+
},
6777
}
6878

6979
var err error
7080
cfg, err = testEnv.Start()
7181
Expect(err).ToNot(HaveOccurred())
7282
Expect(cfg).ToNot(BeNil())
73-
7483
})
7584

7685
var _ = AfterSuite(func() {
7786
By("tearing down the test environment")
7887
Expect(testEnv.Stop()).To(Succeed())
7988
})
89+
90+
func TestMain(m *testing.M) {
91+
// Register required object kinds with global scheme.
92+
if err := machinev1.Install(scheme.Scheme); err != nil {
93+
log.Fatalf("cannot add scheme: %v", err)
94+
}
95+
if err := configv1.Install(scheme.Scheme); err != nil {
96+
log.Fatalf("cannot add scheme: %v", err)
97+
}
98+
exitVal := m.Run()
99+
os.Exit(exitVal)
100+
}

pkg/controller/machineset/machineset_controller_test.go

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,13 @@ import (
2525

2626
machinev1 "github.com/openshift/api/machine/v1beta1"
2727
machinev1resourcebuilder "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder/machine/v1beta1"
28-
testutils "github.com/openshift/machine-api-operator/pkg/util/testing"
2928

29+
"github.com/openshift/cluster-control-plane-machine-set-operator/test/e2e/framework"
30+
testutils "github.com/openshift/machine-api-operator/pkg/util/testing"
3031
corev1 "k8s.io/api/core/v1"
3132
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3233
"sigs.k8s.io/controller-runtime/pkg/client"
34+
"sigs.k8s.io/controller-runtime/pkg/envtest/komega"
3335
"sigs.k8s.io/controller-runtime/pkg/manager"
3436

3537
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
@@ -38,6 +40,7 @@ import (
3840
var _ = Describe("MachineSet Reconciler", func() {
3941
var mgrCtxCancel context.CancelFunc
4042
var mgrStopped chan struct{}
43+
var k komega.Komega
4144
var namespace *corev1.Namespace
4245
var machineSetBuilder machinev1resourcebuilder.MachineSetBuilder
4346
var replicas int32 = int32(2)
@@ -52,6 +55,7 @@ var _ = Describe("MachineSet Reconciler", func() {
5255
Expect(err).NotTo(HaveOccurred())
5356

5457
k8sClient = mgr.GetClient()
58+
k = komega.New(k8sClient)
5559

5660
By("Setting up feature gates")
5761
gate, err := testutils.NewDefaultMutableFeatureGate()
@@ -105,6 +109,11 @@ var _ = Describe("MachineSet Reconciler", func() {
105109
By("Creating the MachineSet")
106110
Expect(k8sClient.Create(ctx, instance)).To(Succeed())
107111

112+
By("Setting the AuthoritativeAPI to MachineAPI")
113+
Eventually(k.UpdateStatus(instance, func() {
114+
instance.Status.AuthoritativeAPI = machinev1.MachineAuthorityMachineAPI
115+
})).Should(Succeed())
116+
108117
machines := &machinev1.MachineList{}
109118
By("Verifying that we have 2 replicas")
110119
Eventually(func() (int, error) {
@@ -133,6 +142,96 @@ var _ = Describe("MachineSet Reconciler", func() {
133142
return ready, nil
134143
}, timeout*3).Should(BeEquivalentTo(replicas))
135144
})
145+
146+
It("Should set the Paused condition appropriately", func() {
147+
instance := machineSetBuilder.
148+
WithName("baz").
149+
WithLabels(map[string]string{"baz": "bar"}).
150+
Build()
151+
152+
By("Creating the MachineSet")
153+
Expect(k8sClient.Create(ctx, instance)).To(Succeed())
154+
155+
By("Setting the AuthoritativeAPI to ClusterAPI")
156+
Eventually(k.UpdateStatus(instance, func() {
157+
instance.Status.AuthoritativeAPI = machinev1.MachineAuthorityClusterAPI
158+
})).Should(Succeed())
159+
160+
By("Verifying that the AuthoritativeAPI is set to Cluster API")
161+
Eventually(k.Object(instance), timeout).Should(HaveField("Status.AuthoritativeAPI", Equal(machinev1.MachineAuthorityClusterAPI)))
162+
163+
By("Verifying the paused condition is approproately set to true")
164+
Eventually(k.Object(instance), timeout).Should(HaveField("Status.Conditions", ContainElement(SatisfyAll(
165+
HaveField("Type", Equal(PausedCondition)),
166+
HaveField("Status", Equal(corev1.ConditionTrue)),
167+
))))
168+
169+
// The condition should remain true whilst transitioning through 'Migrating'
170+
// Run this in a goroutine so we don't block
171+
172+
// Copy the instance before starting the goroutine, to avoid data races
173+
instanceCopy := instance.DeepCopy()
174+
go func() {
175+
defer GinkgoRecover()
176+
framework.RunCheckUntil(
177+
ctx,
178+
// Check that we consistently have the Paused condition true
179+
func(_ context.Context, g framework.GomegaAssertions) bool {
180+
By("Checking that the paused condition is consistently true whilst migrating to MachineAPI")
181+
182+
localInstance := instanceCopy.DeepCopy()
183+
if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(localInstance), localInstance); err != nil {
184+
return g.Expect(err).Should(WithTransform(testutils.IsRetryableAPIError, BeTrue()), "expected temporary error while getting machine: %v", err)
185+
}
186+
187+
return g.Expect(localInstance.Status.Conditions).Should(ContainElement(SatisfyAll(
188+
HaveField("Type", Equal(PausedCondition)),
189+
HaveField("Status", Equal(corev1.ConditionTrue)),
190+
)))
191+
192+
},
193+
// Condition / until function: until we observe the MachineAuthority being MAPI
194+
func(_ context.Context, g framework.GomegaAssertions) bool {
195+
By("Checking that the AuthoritativeAPI is not MachineAPI")
196+
197+
localInstance := instanceCopy.DeepCopy()
198+
if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(localInstance), localInstance); err != nil {
199+
return g.Expect(err).Should(WithTransform(testutils.IsRetryableAPIError, BeTrue()), "expected temporary error while getting machine: %v", err)
200+
}
201+
202+
return g.Expect(localInstance.Status.AuthoritativeAPI).To(Equal(machinev1.MachineAuthorityMachineAPI))
203+
})
204+
}()
205+
206+
By("Transitioning the AuthoritativeAPI though Migrating")
207+
Eventually(k.UpdateStatus(instance, func() {
208+
instance.Status.AuthoritativeAPI = machinev1.MachineAuthorityMigrating
209+
})).Should(Succeed())
210+
211+
By("Updating the AuthoritativeAPI from Migrating to MachineAPI")
212+
Eventually(k.UpdateStatus(instance, func() {
213+
instance.Status.AuthoritativeAPI = machinev1.MachineAuthorityMachineAPI
214+
})).Should(Succeed())
215+
216+
Eventually(k.Object(instance), timeout).Should(HaveField("Status.AuthoritativeAPI", Equal(machinev1.MachineAuthorityMachineAPI)))
217+
218+
By("Verifying the paused condition is approproately set to false")
219+
Eventually(k.Object(instance), timeout).Should(HaveField("Status.Conditions", ContainElement(SatisfyAll(
220+
HaveField("Type", Equal(PausedCondition)),
221+
HaveField("Status", Equal(corev1.ConditionFalse)),
222+
))))
223+
224+
By("Unsetting the AuthoritativeAPI field in the status")
225+
Eventually(k.UpdateStatus(instance, func() {
226+
instance.Status.AuthoritativeAPI = ""
227+
})).Should(Succeed())
228+
229+
By("Verifying the paused condition is still approproately set to false")
230+
Eventually(k.Object(instance), timeout).Should(HaveField("Status.Conditions", ContainElement(SatisfyAll(
231+
HaveField("Type", Equal(PausedCondition)),
232+
HaveField("Status", Equal(corev1.ConditionFalse)),
233+
))))
234+
})
136235
})
137236

138237
func cleanResources(namespace string) error {

0 commit comments

Comments
 (0)