Skip to content

🌱 Enable MS in-place rollout check in e2e test #12943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

🌱 Enable MS in-place rollout check in e2e test #12943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions controlplane/kubeadm/internal/control_plane.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"sigs.k8s.io/cluster-api/controllers/external"
"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/etcd"
"sigs.k8s.io/cluster-api/internal/hooks"
"sigs.k8s.io/cluster-api/internal/util/inplace"
"sigs.k8s.io/cluster-api/util/collections"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/failuredomains"
Expand Down Expand Up @@ -199,12 +200,7 @@ func (c *ControlPlane) MachinesToCompleteTriggerInPlaceUpdate() collections.Mach

// MachinesToCompleteInPlaceUpdate returns Machines that still have to complete their in-place update.
func (c *ControlPlane) MachinesToCompleteInPlaceUpdate() collections.Machines {
return c.Machines.Filter(func(machine *clusterv1.Machine) bool {
// Note: Checking both annotations here to make this slightly more robust.
// Theoretically only checking for IsPending would have been enough.
_, ok := machine.Annotations[clusterv1.UpdateInProgressAnnotation]
return ok || hooks.IsPending(runtimehooksv1.UpdateMachine, machine)
})
return c.Machines.Filter(inplace.IsUpdateInProgress)
}

// FailureDomainWithMostMachines returns the fd with most machines in it and at least one eligible machine in it.
Expand Down
2 changes: 1 addition & 1 deletion docs/book/src/tasks/experimental-features/runtime-sdk/implement-extensions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ created, the extension will detect the associated service and discover the assoc
check the status of the ExtensionConfig. Below is an example of `ExtensionConfig` -

```yaml
apiVersion: runtime.cluster.x-k8s.io/v1alpha1
apiVersion: runtime.cluster.x-k8s.io/v1beta2
kind: ExtensionConfig
metadata:
annotations:
Expand Down
7 changes: 3 additions & 4 deletions docs/proposals/20220221-runtime-SDK.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ behavior is introduced by this proposal:
The Cluster administrator is required to register available Runtime Extension server using the following CR:

```yaml
apiVersion: runtime.cluster.x-k8s.io/v1alpha1
apiVersion: runtime.cluster.x-k8s.io/v1beta2
kind: ExtensionConfig
metadata:
name: "my-amazing-extensions"
Expand Down Expand Up @@ -326,7 +326,7 @@ The `namespaceSelector` will enable targeting of a subset of Clusters.

```yaml

apiVersion: runtime.cluster.x-k8s.io/v1alpha1
apiVersion: runtime.cluster.x-k8s.io/v1beta2
kind: ExtensionConfig
metadata:
name: "my-amazing-extensions"
Expand Down Expand Up @@ -367,8 +367,7 @@ dependencies among Runtime Extensions, being it modeled with something similar t
[systemd unit options](https://www.freedesktop.org/software/systemd/man/systemd.unit.html) or alternative approaches.

The main reason behind that is that such type of feature introduces complexity and creates "pet" like relations across
components making the overall system more fragile. This is also consistent with the [avoid dependencies](#avoid-dependencies)
recommendation above.
components making the overall system more fragile. This is also consistent with the avoid dependencies recommendation.

## Runtime Hooks developer guide (CAPI internals)

Expand Down
2 changes: 1 addition & 1 deletion internal/controllers/machinedeployment/machinedeployment_rollout_rollingupdate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ func (p *rolloutPlanner) reconcileInPlaceUpdateIntent(ctx context.Context) error
}

// Check if the MachineSet can update in place; if not, move to the next MachineSet.
canUpdateMachineSetInPlaceFunc := func(_ *clusterv1.MachineSet) bool { return false }
canUpdateMachineSetInPlaceFunc := func(_ *clusterv1.MachineSet) bool { return true }
Copy link
Member

@fabriziopandini fabriziopandini Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I correct this is temporary to get some test signal, right?

Copy link
Member Author

@sbueringer sbueringer Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Copy link
Member Author

@sbueringer sbueringer Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably won't work atm anyway. Let's see where we are after the CanUpdateMachineSet PR and if the RX is disabled cleanly enough for other test cases or if we want to introduce some additional handling

Copy link
Member Author

@sbueringer sbueringer Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will only enable in-place for the in-place e2e test

if p.overrideCanUpdateMachineSetInPlace != nil {
canUpdateMachineSetInPlaceFunc = p.overrideCanUpdateMachineSetInPlace
}
Expand Down
5 changes: 4 additions & 1 deletion internal/controllers/machinedeployment/machinedeployment_rollout_rollingupdate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1753,7 +1753,10 @@ func runRollingUpdateTestCase(ctx context.Context, t *testing.T, tt rollingUpdat

// Running a small subset of MD reconcile (the rollout logic and a bit of setReplicas)
p := newRolloutPlanner()
p.overrideCanUpdateMachineSetInPlace = tt.overrideCanUpdateMachineSetInPlaceFunc
p.overrideCanUpdateMachineSetInPlace = func(_ *clusterv1.MachineSet) bool { return false }
if tt.overrideCanUpdateMachineSetInPlaceFunc != nil {
p.overrideCanUpdateMachineSetInPlace = tt.overrideCanUpdateMachineSetInPlaceFunc
}
p.overrideComputeDesiredMS = func(ctx context.Context, deployment *clusterv1.MachineDeployment, currentNewMS *clusterv1.MachineSet) (*clusterv1.MachineSet, error) {
log := ctrl.LoggerFrom(ctx)
desiredNewMS := currentNewMS
Expand Down
11 changes: 8 additions & 3 deletions test/e2e/cluster_in_place_update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,13 @@ func ClusterInPlaceUpdateSpec(ctx context.Context, inputGetter func() ClusterInP

// Doing multiple in-place updates for additional coverage.
filePath := "/tmp/test"
for i, fileContent := range []string{"first in-place update", "second in-place update"} {
for i, fileContent := range []string{
"first in-place update",
"second in-place update",
"third in-place update",
"fourth in-place update",
"five in-place update",
} {
Byf("[%d] Trigger in-place update by modifying the files variable", i)

originalCluster := cluster.DeepCopy()
Expand Down Expand Up @@ -246,8 +252,7 @@ func ClusterInPlaceUpdateSpec(ctx context.Context, inputGetter func() ClusterInP
// Ensure only in-place updates were executed and no Machine was re-created.
machineObjectsAfterInPlaceUpdate = getMachineObjects(ctx, g, mgmtClient, cluster)
g.Expect(machineNames(machineObjectsAfterInPlaceUpdate.ControlPlaneMachines)).To(Equal(machineNames(machineObjectsBeforeInPlaceUpdate.ControlPlaneMachines)))
// TODO(in-place): enable once MD/MS/Machine controller PRs are merged
// g.Expect(machineNames(machineObjectsAfterInPlaceUpdate.WorkerMachines)).To(Equal(machineNames(machineObjectsBeforeInPlaceUpdate.WorkerMachines)))
g.Expect(machineNames(machineObjectsAfterInPlaceUpdate.WorkerMachines)).To(Equal(machineNames(machineObjectsBeforeInPlaceUpdate.WorkerMachines)))
}, input.E2EConfig.GetIntervals(specName, "wait-control-plane")...).Should(Succeed())

// Update machineObjectsBeforeInPlaceUpdate for the next round of in-place update.
Expand Down
4 changes: 2 additions & 2 deletions test/extension/config/tilt/extensionconfig.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
apiVersion: runtime.cluster.x-k8s.io/v1alpha1
apiVersion: runtime.cluster.x-k8s.io/v1beta2
kind: ExtensionConfig
metadata:
annotations:
Expand All @@ -17,4 +17,4 @@ spec:
- key: kubernetes.io/metadata.name
operator: In
values:
- default # Note: this assumes the test extension is used by Cluster in the default namespace only
- default # Note: this assumes the test extension is used by Cluster in the default namespace only
3 changes: 2 additions & 1 deletion test/extension/handlers/inplaceupdate/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"gomodules.xyz/jsonpatch/v2"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apimachinery/pkg/util/rand"
"k8s.io/klog/v2"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -198,7 +199,7 @@ func (h *ExtensionHandlers) DoUpdateMachine(ctx context.Context, req *runtimehoo
// Note: We are intentionally not actually applying any in-place changes we are just faking them,
// which is good enough for test purposes.
if firstTimeCalled, ok := h.state.Load(key); ok {
if time.Since(firstTimeCalled.(time.Time)) > 20*time.Second {
if time.Since(firstTimeCalled.(time.Time)) > time.Duration(20+rand.Intn(10))*time.Second {
h.state.Delete(key)
resp.Status = runtimehooksv1.ResponseStatusSuccess
resp.Message = "In-place update is done"
Expand Down