diff --git a/.github/workflows/publish-docker.yml b/.github/workflows/publish-docker.yml index 3825b6db8..c5915846b 100644 --- a/.github/workflows/publish-docker.yml +++ b/.github/workflows/publish-docker.yml @@ -87,7 +87,7 @@ jobs: version: latest endpoint: builders # self-hosted - name: Login to GHCR - if: github.event_name != 'pull_request' + if: github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'ok-to-image') uses: docker/login-action@v4 with: registry: ghcr.io @@ -99,7 +99,7 @@ jobs: with: context: . platforms: linux/amd64,linux/arm64 - push: ${{ github.event_name != 'pull_request' }} + push: ${{ github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'ok-to-image') }} tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} target: ${{ matrix.image.target }} diff --git a/api/compute/v1alpha1/common.go b/api/compute/v1alpha1/common.go index 661711ed0..9714e2d62 100644 --- a/api/compute/v1alpha1/common.go +++ b/api/compute/v1alpha1/common.go @@ -9,6 +9,10 @@ import ( corev1 "k8s.io/api/core/v1" ) +const ( + NamespaceMachinePoolLease = "ironcore-machinepool-lease" +) + const ( MachineMachinePoolRefNameField = "spec.machinePoolRef.name" MachineMachineClassRefNameField = "spec.machineClassRef.name" diff --git a/api/compute/v1alpha1/conditions.go b/api/compute/v1alpha1/conditions.go new file mode 100644 index 000000000..3110139aa --- /dev/null +++ b/api/compute/v1alpha1/conditions.go @@ -0,0 +1,46 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import ( + "slices" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// FindMachinePoolCondition returns a pointer to the condition of the given type, +// or nil if no condition of that type is present. +func FindMachinePoolCondition(conditions []MachinePoolCondition, typ MachinePoolConditionType) *MachinePoolCondition { + idx := slices.IndexFunc(conditions, func(cond MachinePoolCondition) bool { + return cond.Type == typ + }) + if idx < 0 { + return nil + } + return &conditions[idx] +} + +// SetMachinePoolCondition inserts or updates a condition of the given type in the +// conditions slice. LastUpdateTime is always set to now. LastTransitionTime is set +// to now only when the condition is newly inserted or its Status differs from the +// previous value. +func SetMachinePoolCondition(conditions []MachinePoolCondition, cond MachinePoolCondition) []MachinePoolCondition { + idx := slices.IndexFunc(conditions, func(c MachinePoolCondition) bool { + return c.Type == cond.Type + }) + + cond.LastUpdateTime = metav1.Now() + + if idx < 0 || conditions[idx].Status != cond.Status { + cond.LastTransitionTime = metav1.Now() + } else { + cond.LastTransitionTime = conditions[idx].LastTransitionTime + } + + if idx < 0 { + return append(conditions, cond) + } + conditions[idx] = cond + return conditions +} diff --git a/api/compute/v1alpha1/conditions_test.go b/api/compute/v1alpha1/conditions_test.go new file mode 100644 index 000000000..028f4f2ae --- /dev/null +++ b/api/compute/v1alpha1/conditions_test.go @@ -0,0 +1,100 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1_test + +import ( + "time" + + computev1alpha1 "github.com/ironcore-dev/ironcore/api/compute/v1alpha1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" + "github.com/onsi/gomega/types" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("Conditions", func() { + DescribeTable("FindMachinePoolCondition", + func(conds []computev1alpha1.MachinePoolCondition, condType computev1alpha1.MachinePoolConditionType, match types.GomegaMatcher) { + Expect(computev1alpha1.FindMachinePoolCondition(conds, condType)).To(match) + }, + Entry("returns the matching condition", + []computev1alpha1.MachinePoolCondition{ + {Type: "Other", Status: corev1.ConditionTrue}, + {Type: computev1alpha1.MachinePoolReady, Status: corev1.ConditionFalse, Reason: "X"}, + }, + computev1alpha1.MachinePoolReady, + PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(computev1alpha1.MachinePoolReady), + "Reason": Equal("X"), + })), + ), + Entry("returns nil when no condition of the given type is present", + []computev1alpha1.MachinePoolCondition{{Type: "Other"}}, + computev1alpha1.MachinePoolReady, + BeNil(), + ), + Entry("returns nil for an empty slice", + []computev1alpha1.MachinePoolCondition{}, + computev1alpha1.MachinePoolReady, + BeNil(), + ), + ) + + Describe("SetMachinePoolCondition", func() { + It("should append the condition when it is absent", func() { + out := computev1alpha1.SetMachinePoolCondition(nil, computev1alpha1.MachinePoolCondition{ + Type: computev1alpha1.MachinePoolReady, + Status: corev1.ConditionTrue, + }) + + Expect(out).To(HaveLen(1)) + Expect(out[0].Type).To(Equal(computev1alpha1.MachinePoolReady)) + + By("setting LastTransitionTime on first append") + Expect(out[0].LastTransitionTime.IsZero()).To(BeFalse()) + + By("setting LastUpdateTime on first append") + Expect(out[0].LastUpdateTime.IsZero()).To(BeFalse()) + }) + + It("should update in place without advancing LastTransitionTime when status is unchanged", func() { + earlier := metav1.NewTime(time.Now().Add(-time.Hour)) + in := []computev1alpha1.MachinePoolCondition{{ + Type: computev1alpha1.MachinePoolReady, + Status: corev1.ConditionTrue, + LastTransitionTime: earlier, + }} + + out := computev1alpha1.SetMachinePoolCondition(in, computev1alpha1.MachinePoolCondition{ + Type: computev1alpha1.MachinePoolReady, + Status: corev1.ConditionTrue, // same status + Message: "still ready", + }) + + By("preserving LastTransitionTime when status is unchanged") + Expect(out[0].LastTransitionTime.Equal(&earlier)).To(BeTrue()) + + By("advancing LastUpdateTime") + Expect(out[0].LastUpdateTime.IsZero()).To(BeFalse()) + }) + + It("should advance LastTransitionTime when the status changes", func() { + earlier := metav1.NewTime(time.Now().Add(-time.Hour)) + in := []computev1alpha1.MachinePoolCondition{{ + Type: computev1alpha1.MachinePoolReady, + Status: corev1.ConditionTrue, + LastTransitionTime: earlier, + }} + + out := computev1alpha1.SetMachinePoolCondition(in, computev1alpha1.MachinePoolCondition{ + Type: computev1alpha1.MachinePoolReady, + Status: corev1.ConditionFalse, + }) + + Expect(out[0].LastTransitionTime.Equal(&earlier)).To(BeFalse()) + }) + }) +}) diff --git a/api/compute/v1alpha1/machinepool_types.go b/api/compute/v1alpha1/machinepool_types.go index 421b95cc4..1c5a966a9 100644 --- a/api/compute/v1alpha1/machinepool_types.go +++ b/api/compute/v1alpha1/machinepool_types.go @@ -85,6 +85,11 @@ type MachinePoolAddress struct { // MachinePoolConditionType is a type a MachinePoolCondition can have. type MachinePoolConditionType string +const ( + // MachinePoolReady means the machine pool is healthy and ready to accept machines. + MachinePoolReady MachinePoolConditionType = "Ready" +) + // MachinePoolCondition is one of the conditions of a MachinePool. type MachinePoolCondition struct { // Type is the type of the condition. @@ -97,6 +102,8 @@ type MachinePoolCondition struct { Message string `json:"message"` // ObservedGeneration represents the .metadata.generation that the condition was set based upon. ObservedGeneration int64 `json:"observedGeneration,omitempty"` + // LastUpdateTime is the last time this condition was updated. + LastUpdateTime metav1.Time `json:"lastUpdateTime,omitempty"` // LastTransitionTime is the last time the status of a condition has transitioned from one state to another. LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` } diff --git a/api/compute/v1alpha1/v1alpha1_suite_test.go b/api/compute/v1alpha1/v1alpha1_suite_test.go new file mode 100644 index 000000000..af8aa7bc7 --- /dev/null +++ b/api/compute/v1alpha1/v1alpha1_suite_test.go @@ -0,0 +1,16 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestV1Alpha1(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Compute v1alpha1 Suite") +} diff --git a/api/compute/v1alpha1/zz_generated.deepcopy.go b/api/compute/v1alpha1/zz_generated.deepcopy.go index 5808efb15..6fad42864 100644 --- a/api/compute/v1alpha1/zz_generated.deepcopy.go +++ b/api/compute/v1alpha1/zz_generated.deepcopy.go @@ -365,6 +365,7 @@ func (in *MachinePoolAddress) DeepCopy() *MachinePoolAddress { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachinePoolCondition) DeepCopyInto(out *MachinePoolCondition) { *out = *in + in.LastUpdateTime.DeepCopyInto(&out.LastUpdateTime) in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) return } diff --git a/client-go/applyconfigurations/compute/v1alpha1/machinepoolcondition.go b/client-go/applyconfigurations/compute/v1alpha1/machinepoolcondition.go index 6ca1a2f3e..ac89e33f3 100644 --- a/client-go/applyconfigurations/compute/v1alpha1/machinepoolcondition.go +++ b/client-go/applyconfigurations/compute/v1alpha1/machinepoolcondition.go @@ -26,6 +26,8 @@ type MachinePoolConditionApplyConfiguration struct { Message *string `json:"message,omitempty"` // ObservedGeneration represents the .metadata.generation that the condition was set based upon. ObservedGeneration *int64 `json:"observedGeneration,omitempty"` + // LastUpdateTime is the last time this condition was updated. + LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` // LastTransitionTime is the last time the status of a condition has transitioned from one state to another. LastTransitionTime *metav1.Time `json:"lastTransitionTime,omitempty"` } @@ -76,6 +78,14 @@ func (b *MachinePoolConditionApplyConfiguration) WithObservedGeneration(value in return b } +// WithLastUpdateTime sets the LastUpdateTime field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the LastUpdateTime field is set to the value of the last call. +func (b *MachinePoolConditionApplyConfiguration) WithLastUpdateTime(value metav1.Time) *MachinePoolConditionApplyConfiguration { + b.LastUpdateTime = &value + return b +} + // WithLastTransitionTime sets the LastTransitionTime field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the LastTransitionTime field is set to the value of the last call. diff --git a/client-go/openapi/zz_generated.openapi.go b/client-go/openapi/zz_generated.openapi.go index 45536737c..10c866349 100644 --- a/client-go/openapi/zz_generated.openapi.go +++ b/client-go/openapi/zz_generated.openapi.go @@ -1295,6 +1295,12 @@ func schema_ironcore_api_compute_v1alpha1_MachinePoolCondition(ref common.Refere Format: "int64", }, }, + "lastUpdateTime": { + SchemaProps: spec.SchemaProps{ + Description: "LastUpdateTime is the last time this condition was updated.", + Ref: ref(metav1.Time{}.OpenAPIModelName()), + }, + }, "lastTransitionTime": { SchemaProps: spec.SchemaProps{ Description: "LastTransitionTime is the last time the status of a condition has transitioned from one state to another.", diff --git a/cmd/ironcore-controller-manager/main.go b/cmd/ironcore-controller-manager/main.go index 053a5f894..ece7e9eaf 100644 --- a/cmd/ironcore-controller-manager/main.go +++ b/cmd/ironcore-controller-manager/main.go @@ -42,6 +42,8 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -50,6 +52,7 @@ import ( computev1alpha1 "github.com/ironcore-dev/ironcore/api/compute/v1alpha1" ipamv1alpha1 "github.com/ironcore-dev/ironcore/api/ipam/v1alpha1" storagev1alpha1 "github.com/ironcore-dev/ironcore/api/storage/v1alpha1" + coordinationv1 "k8s.io/api/coordination/v1" //+kubebuilder:scaffold:imports ) @@ -65,6 +68,7 @@ const ( machineSchedulerController = "machinescheduler" machineEvictionController = "machineeviction" machineClassController = "machineclass" + machinePoolLifecycleController = "machinepoollifecycle" // storage controllers bucketScheduler = "bucketscheduler" @@ -115,6 +119,7 @@ func main() { var volumeBindTimeout time.Duration var virtualIPBindTimeout time.Duration var networkInterfaceBindTimeout time.Duration + var machinePoolLifecycleGracePeriod time.Duration var tlsOpts []func(*tls.Config) flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+ "Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.") @@ -135,6 +140,7 @@ func main() { flag.DurationVar(&volumeBindTimeout, "volume-bind-timeout", 10*time.Second, "Time to wait until considering a volume bind to be failed.") flag.DurationVar(&virtualIPBindTimeout, "virtual-ip-bind-timeout", 10*time.Second, "Time to wait until considering a virtual ip bind to be failed.") flag.DurationVar(&networkInterfaceBindTimeout, "network-interface-bind-timeout", 10*time.Second, "Time to wait until considering a network interface bind to be failed.") + flag.DurationVar(&machinePoolLifecycleGracePeriod, "machine-pool-lifecycle-grace-period", 50*time.Second, "Grace period without a heartbeat before a machine pool's Ready condition is marked Unknown.") controllers := switches.New( // compute controllers @@ -142,6 +148,7 @@ func main() { machineEphemeralVolumeController, machineSchedulerController, machineClassController, + machinePoolLifecycleController, // storage controllers bucketScheduler, @@ -258,6 +265,19 @@ func main() { PprofBindAddress: pprofAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "d0ae00be.ironcore.dev", + Cache: cache.Options{ + ByObject: map[client.Object]cache.ByObject{ + // The MachinePool lifecycle controller watches Leases only in + // NamespaceMachinePoolLease. Scoping the cache here keeps the + // underlying informer namespaced so the controller's RBAC can + // stay namespaced too. + &coordinationv1.Lease{}: { + Namespaces: map[string]cache.Config{ + computev1alpha1.NamespaceMachinePoolLease: {}, + }, + }, + }, + }, }) if err != nil { setupLog.Error(err, "unable to create manager") @@ -332,6 +352,16 @@ func main() { } } + if controllers.Enabled(machinePoolLifecycleController) { + if err := (&computecontrollers.MachinePoolLifecycleReconciler{ + Client: mgr.GetClient(), + GracePeriod: machinePoolLifecycleGracePeriod, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "MachinePoolLifecycle") + os.Exit(1) + } + } + // storage controllers if controllers.Enabled(bucketScheduler) { diff --git a/config/apiserver/rbac/machinepool_role.yaml b/config/apiserver/rbac/machinepool_role.yaml index 81343cbb6..1140cdda2 100644 --- a/config/apiserver/rbac/machinepool_role.yaml +++ b/config/apiserver/rbac/machinepool_role.yaml @@ -140,3 +140,21 @@ rules: - patch - update - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: compute.ironcore.dev:system:machinepools + namespace: ironcore-machinepool-lease +rules: +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create + - get + - list + - patch + - update + - watch diff --git a/config/apiserver/standalone-etcdless/kustomization.yaml b/config/apiserver/standalone-etcdless/kustomization.yaml index 65fa81bc3..5e4d3cb84 100644 --- a/config/apiserver/standalone-etcdless/kustomization.yaml +++ b/config/apiserver/standalone-etcdless/kustomization.yaml @@ -7,3 +7,4 @@ kind: Kustomization resources: - ../etcdless - ../../namespaces/ironcore-system + - ../../namespaces/machinepool-lease diff --git a/config/apiserver/standalone/kustomization.yaml b/config/apiserver/standalone/kustomization.yaml index 486879a4f..05e49617a 100644 --- a/config/apiserver/standalone/kustomization.yaml +++ b/config/apiserver/standalone/kustomization.yaml @@ -8,3 +8,4 @@ kind: Kustomization resources: - ../default - ../../namespaces/ironcore-system + - ../../namespaces/machinepool-lease diff --git a/config/controller/rbac/role.yaml b/config/controller/rbac/role.yaml index 772748bc9..a8b5a498b 100644 --- a/config/controller/rbac/role.yaml +++ b/config/controller/rbac/role.yaml @@ -74,6 +74,7 @@ rules: - compute.ironcore.dev resources: - machineclasses/status + - machinepools/status - machines/status verbs: - get diff --git a/config/controller/standalone/kustomization.yaml b/config/controller/standalone/kustomization.yaml index 0ecde42d3..7a91ddbe2 100644 --- a/config/controller/standalone/kustomization.yaml +++ b/config/controller/standalone/kustomization.yaml @@ -9,3 +9,4 @@ kind: Kustomization resources: - ../default - ../../namespaces/ironcore-system + - ../../namespaces/machinepool-lease diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 4b737f4a6..9827c6bba 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -8,3 +8,4 @@ resources: - ../apiserver/default - ../controller/default - ../namespaces/ironcore-system + - ../namespaces/machinepool-lease diff --git a/config/etcdless/kustomization.yaml b/config/etcdless/kustomization.yaml index 51a59a8c9..552f82640 100644 --- a/config/etcdless/kustomization.yaml +++ b/config/etcdless/kustomization.yaml @@ -8,3 +8,4 @@ resources: - ../apiserver/etcdless - ../controller/default - ../namespaces/ironcore-system + - ../namespaces/machinepool-lease diff --git a/config/machinepoollet-broker/poollet-rbac/role.yaml b/config/machinepoollet-broker/poollet-rbac/role.yaml index c28290788..3046a4851 100644 --- a/config/machinepoollet-broker/poollet-rbac/role.yaml +++ b/config/machinepoollet-broker/poollet-rbac/role.yaml @@ -140,3 +140,21 @@ rules: - patch - update - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: manager-role + namespace: ironcore-machinepool-lease +rules: +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create + - get + - list + - patch + - update + - watch diff --git a/config/namespaces/machinepool-lease/controller_role.yaml b/config/namespaces/machinepool-lease/controller_role.yaml new file mode 100644 index 000000000..633b2ffb4 --- /dev/null +++ b/config/namespaces/machinepool-lease/controller_role.yaml @@ -0,0 +1,20 @@ +# SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +# SPDX-License-Identifier: Apache-2.0 + +# Lets the ironcore controller manager observe pool leases. The lifecycle +# controller (see internal/controllers/compute/machinepool_lifecycle_controller.go) +# watches and reads these leases to detect failed pools. +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: ironcore-controller-manager:lease-reader + namespace: ironcore-machinepool-lease +rules: +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - get + - list + - watch diff --git a/config/namespaces/machinepool-lease/controller_rolebinding.yaml b/config/namespaces/machinepool-lease/controller_rolebinding.yaml new file mode 100644 index 000000000..a5605169b --- /dev/null +++ b/config/namespaces/machinepool-lease/controller_rolebinding.yaml @@ -0,0 +1,21 @@ +# SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +# SPDX-License-Identifier: Apache-2.0 + +# Cross-namespace binding: the controller runs in ironcore-system but needs to +# read leases in ironcore-machinepool-lease. The subject name is hard-coded with +# the `ironcore-` prefix because this binding lives outside the layer that +# applies `namePrefix: ironcore-`. If that prefix is ever changed in +# config/controller/default, this name must be updated too. +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: ironcore-controller-manager:lease-reader + namespace: ironcore-machinepool-lease +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: ironcore-controller-manager:lease-reader +subjects: + - kind: ServiceAccount + name: ironcore-controller-manager + namespace: ironcore-system diff --git a/config/namespaces/machinepool-lease/kustomization.yaml b/config/namespaces/machinepool-lease/kustomization.yaml new file mode 100644 index 000000000..555482660 --- /dev/null +++ b/config/namespaces/machinepool-lease/kustomization.yaml @@ -0,0 +1,9 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +resources: + - namespace.yaml + - poollet_role.yaml + - poollet_rolebinding.yaml + - controller_role.yaml + - controller_rolebinding.yaml diff --git a/config/namespaces/machinepool-lease/namespace.yaml b/config/namespaces/machinepool-lease/namespace.yaml new file mode 100644 index 000000000..dfd84627a --- /dev/null +++ b/config/namespaces/machinepool-lease/namespace.yaml @@ -0,0 +1,10 @@ +# SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +# SPDX-License-Identifier: Apache-2.0 + +# Cluster-shared namespace where every machine poollet renews its pool Lease. +# Lifecycle controllers in the ironcore-system namespace watch leases here to +# detect failed pools. +apiVersion: v1 +kind: Namespace +metadata: + name: ironcore-machinepool-lease diff --git a/config/namespaces/machinepool-lease/poollet_role.yaml b/config/namespaces/machinepool-lease/poollet_role.yaml new file mode 100644 index 000000000..9695ef8bf --- /dev/null +++ b/config/namespaces/machinepool-lease/poollet_role.yaml @@ -0,0 +1,24 @@ +# SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +# SPDX-License-Identifier: Apache-2.0 + +# Role granting the rights every machine poollet needs in the lease namespace +# to renew its pool lease. Bound by the matching RoleBinding in this directory +# to the `compute.ironcore.dev:system:machinepools` Group, which every poollet +# joins via its client cert organization. +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: compute.ironcore.dev:system:machinepools + namespace: ironcore-machinepool-lease +rules: +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create + - get + - list + - patch + - update + - watch diff --git a/config/namespaces/machinepool-lease/poollet_rolebinding.yaml b/config/namespaces/machinepool-lease/poollet_rolebinding.yaml new file mode 100644 index 000000000..2b87d41ed --- /dev/null +++ b/config/namespaces/machinepool-lease/poollet_rolebinding.yaml @@ -0,0 +1,19 @@ +# SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +# SPDX-License-Identifier: Apache-2.0 + +# Binds the lease-renewal Role to every authenticated machine poollet via the +# `compute.ironcore.dev:system:machinepools` Group, which every poollet joins +# through its client cert organization (see api/compute/v1alpha1/common.go). +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: compute.ironcore.dev:system:machinepools + namespace: ironcore-machinepool-lease +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: compute.ironcore.dev:system:machinepools +subjects: + - kind: Group + name: compute.ironcore.dev:system:machinepools + apiGroup: rbac.authorization.k8s.io diff --git a/go.mod b/go.mod index 0773c78f8..164ced4de 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/go-chi/chi/v5 v5.3.1 github.com/go-logr/logr v1.4.3 github.com/google/go-cmp v0.7.0 + github.com/google/uuid v1.6.0 github.com/ironcore-dev/controller-utils v0.12.0 github.com/onsi/ginkgo/v2 v2.32.0 github.com/onsi/gomega v1.42.1 @@ -77,7 +78,6 @@ require ( github.com/google/cel-go v0.26.0 // indirect github.com/google/gnostic-models v0.7.0 // indirect github.com/google/pprof v0.0.0-20260402051712-545e8a4df936 // indirect - github.com/google/uuid v1.6.0 // indirect github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 // indirect diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 3710ecac7..30cd48b80 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -45,7 +45,7 @@ declare -a GOMODS=( ) echo "Setting permissions for files of relevant go modules to 644" for MOD in "${GOMODS[@]}"; do - find "$(go list -json -m -u "${MOD}" | jq -r '.Dir')" -type f -exec chmod 644 -- {} + + find "$(go list -json -m -u "${MOD}" | jq -r '.Dir')" -type f -exec chmod 644 {} + done echo "Generating ${blue}openapi${normal}" diff --git a/internal/apis/compute/machinepool_types.go b/internal/apis/compute/machinepool_types.go index 891de8f71..ac9511c69 100644 --- a/internal/apis/compute/machinepool_types.go +++ b/internal/apis/compute/machinepool_types.go @@ -97,6 +97,8 @@ type MachinePoolCondition struct { Message string // ObservedGeneration represents the .metadata.generation that the condition was set based upon. ObservedGeneration int64 + // LastUpdateTime is the last time this condition was updated. + LastUpdateTime metav1.Time // LastTransitionTime is the last time the status of a condition has transitioned from one state to another. LastTransitionTime metav1.Time } diff --git a/internal/apis/compute/v1alpha1/zz_generated.conversion.go b/internal/apis/compute/v1alpha1/zz_generated.conversion.go index e89dadbea..2231978bf 100644 --- a/internal/apis/compute/v1alpha1/zz_generated.conversion.go +++ b/internal/apis/compute/v1alpha1/zz_generated.conversion.go @@ -713,6 +713,7 @@ func autoConvert_v1alpha1_MachinePoolCondition_To_compute_MachinePoolCondition(i out.Reason = in.Reason out.Message = in.Message out.ObservedGeneration = in.ObservedGeneration + out.LastUpdateTime = in.LastUpdateTime out.LastTransitionTime = in.LastTransitionTime return nil } @@ -728,6 +729,7 @@ func autoConvert_compute_MachinePoolCondition_To_v1alpha1_MachinePoolCondition(i out.Reason = in.Reason out.Message = in.Message out.ObservedGeneration = in.ObservedGeneration + out.LastUpdateTime = in.LastUpdateTime out.LastTransitionTime = in.LastTransitionTime return nil } diff --git a/internal/apis/compute/zz_generated.deepcopy.go b/internal/apis/compute/zz_generated.deepcopy.go index 52154d393..1125157b4 100644 --- a/internal/apis/compute/zz_generated.deepcopy.go +++ b/internal/apis/compute/zz_generated.deepcopy.go @@ -365,6 +365,7 @@ func (in *MachinePoolAddress) DeepCopy() *MachinePoolAddress { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachinePoolCondition) DeepCopyInto(out *MachinePoolCondition) { *out = *in + in.LastUpdateTime.DeepCopyInto(&out.LastUpdateTime) in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) return } diff --git a/internal/controllers/compute/machinepool_lifecycle_controller.go b/internal/controllers/compute/machinepool_lifecycle_controller.go new file mode 100644 index 000000000..28a6e6816 --- /dev/null +++ b/internal/controllers/compute/machinepool_lifecycle_controller.go @@ -0,0 +1,224 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package compute + +import ( + "context" + "fmt" + "sync" + "time" + + "github.com/go-logr/logr" + computev1alpha1 "github.com/ironcore-dev/ironcore/api/compute/v1alpha1" + coordinationv1 "k8s.io/api/coordination/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +type MachinePoolLifecycleReconciler struct { + client.Client + GracePeriod time.Duration + + healthDataMu sync.RWMutex + healthData map[string]*MachinePoolHealth +} + +type MachinePoolHealth struct { + lastChangeDetectedTime time.Time + readyCondition *computev1alpha1.MachinePoolCondition + leaseRenewTime time.Time +} + +// Cluster-scope RBAC for the MachinePool lifecycle controller. +// Lease access in NamespaceMachinePoolLease is granted by a hand-written +// namespace-scoped Role+RoleBinding under config/namespaces/machinepool-lease/, +// and the manager's cache is scoped to that namespace, so no cluster-wide +// lease permissions are required here. +//+kubebuilder:rbac:groups=compute.ironcore.dev,resources=machinepools,verbs=get;list;watch +//+kubebuilder:rbac:groups=compute.ironcore.dev,resources=machinepools/status,verbs=get;update;patch + +func (r *MachinePoolLifecycleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := ctrl.LoggerFrom(ctx) + + machinePool := &computev1alpha1.MachinePool{} + if err := r.Get(ctx, req.NamespacedName, machinePool); err != nil { + if apierrors.IsNotFound(err) { + r.deleteMachinePoolHealth(req.Name) + return ctrl.Result{}, nil + } + return ctrl.Result{}, err + } + + return r.reconcileExists(ctx, log, machinePool) +} + +func (r *MachinePoolLifecycleReconciler) getMachinePoolHealth(machinePoolName string) *MachinePoolHealth { + r.healthDataMu.RLock() + defer r.healthDataMu.RUnlock() + + return r.healthData[machinePoolName] +} + +func (r *MachinePoolLifecycleReconciler) setMachinePoolHealth(machinePoolName string, machinePoolHealth *MachinePoolHealth) { + r.healthDataMu.Lock() + defer r.healthDataMu.Unlock() + + if r.healthData == nil { + r.healthData = make(map[string]*MachinePoolHealth) + } + + r.healthData[machinePoolName] = machinePoolHealth +} + +func (r *MachinePoolLifecycleReconciler) deleteMachinePoolHealth(machinePoolName string) { + r.healthDataMu.Lock() + defer r.healthDataMu.Unlock() + + delete(r.healthData, machinePoolName) +} + +func getPreviousHealthValues(machinePoolHealth *MachinePoolHealth) (prevReadyCondition *computev1alpha1.MachinePoolCondition, prevLeaseRenewTime *time.Time) { + if machinePoolHealth != nil { + prevReadyCondition = machinePoolHealth.readyCondition + if !machinePoolHealth.leaseRenewTime.IsZero() { + prevLeaseRenewTime = &machinePoolHealth.leaseRenewTime + } + } + return prevReadyCondition, prevLeaseRenewTime +} + +func (r *MachinePoolLifecycleReconciler) getCurrentLeaseRenewTime(ctx context.Context, machinePoolName string) (*time.Time, error) { + lease := &coordinationv1.Lease{} + if err := r.Get(ctx, client.ObjectKey{ + Namespace: computev1alpha1.NamespaceMachinePoolLease, + Name: machinePoolName, + }, lease); err != nil { + if !apierrors.IsNotFound(err) { + return nil, fmt.Errorf("getting machine pool lease: %w", err) + } + return nil, nil + } + if lease.Spec.RenewTime != nil { + return ptr.To(lease.Spec.RenewTime.Time), nil + } + return nil, nil +} + +// leaseRenewTimeChanged reports whether two optional lease renew times differ, +// using time.Time.Equal to avoid false positives from monotonic clock or +// location pointer differences. +func leaseRenewTimeChanged(a, b *time.Time) bool { + if a == nil && b == nil { + return false + } + if a == nil || b == nil { + return true + } + return !a.Equal(*b) +} + +// readyConditionChanged reports whether two optional ready conditions differ, +// using ObservedGeneration to detect changes even if the Status is the same. +// Used in edge-case scenarios where the Lease is not updated but the Ready condition is still updated, +// e.g. when the MachinePool controller is restarted and re-sets the Ready condition. +func readyConditionChanged(a, b *computev1alpha1.MachinePoolCondition) bool { + if a == nil && b == nil { + return false + } + if a == nil || b == nil { + return true + } + // ObservedGeneration is set to the MachinePool's generation when the condition is updated in the MachinePool controller, + // so we can use it to detect changes in the condition even if the Status is the same. + return a.Status != b.Status || a.ObservedGeneration != b.ObservedGeneration +} + +func (r *MachinePoolLifecycleReconciler) reconcileExists(ctx context.Context, log logr.Logger, machinePool *computev1alpha1.MachinePool) (ctrl.Result, error) { + now := time.Now() + prev := r.getMachinePoolHealth(machinePool.Name) + prevReadyCondition, prevLeaseRenewTime := getPreviousHealthValues(prev) + + currentReadyCondition := computev1alpha1.FindMachinePoolCondition(machinePool.Status.Conditions, computev1alpha1.MachinePoolReady) + currentLeaseRenewTime, err := r.getCurrentLeaseRenewTime(ctx, machinePool.Name) + if err != nil { + return ctrl.Result{}, err + } + + changed := leaseRenewTimeChanged(prevLeaseRenewTime, currentLeaseRenewTime) || readyConditionChanged(prevReadyCondition, currentReadyCondition) + + next := &MachinePoolHealth{ + readyCondition: currentReadyCondition, + leaseRenewTime: ptr.Deref(currentLeaseRenewTime, time.Time{}), + } + + switch { + case prev == nil: + log.V(1).Info("First observation of machine pool") + next.lastChangeDetectedTime = now + case changed: + log.V(1).Info("Lease or ready condition changed") + next.lastChangeDetectedTime = now + default: + next.lastChangeDetectedTime = prev.lastChangeDetectedTime + + if time.Since(prev.lastChangeDetectedTime) > r.GracePeriod { + if currentReadyCondition != nil && currentReadyCondition.Status == corev1.ConditionUnknown { + log.V(2).Info("Grace period exceeded, ready condition already unknown, no patch needed", + "gracePeriod", r.GracePeriod, "lastChangeDetected", prev.lastChangeDetectedTime) + } else { + log.Info("Grace period exceeded without health update, marking machine pool status unknown", + "gracePeriod", r.GracePeriod, "lastChangeDetected", prev.lastChangeDetectedTime) + patch := client.StrategicMergeFrom(machinePool.DeepCopy()) + newReadyCondition := computev1alpha1.MachinePoolCondition{ + Type: computev1alpha1.MachinePoolReady, + Status: corev1.ConditionUnknown, + Reason: "MachinePoolStatusUnknown", + Message: "machinepoollet stopped posting machine pool status.", + ObservedGeneration: machinePool.Generation, + } + machinePool.Status.Conditions = computev1alpha1.SetMachinePoolCondition(machinePool.Status.Conditions, newReadyCondition) + + if err := r.Status().Patch(ctx, machinePool, patch); err != nil { + // On patch failure, leave health state untouched so the next reconcile retries. + return ctrl.Result{}, fmt.Errorf("error patching: %w", err) + } + next.readyCondition = &newReadyCondition + } + } else { + log.V(3).Info("No change, still within grace period", + "gracePeriod", r.GracePeriod, "elapsed", time.Since(prev.lastChangeDetectedTime)) + } + } + + r.setMachinePoolHealth(machinePool.Name, next) + + requeueAfter := time.Until(next.lastChangeDetectedTime.Add(r.GracePeriod)) + if requeueAfter <= 0 { + // Grace period has already expired, requeue after the full grace period to avoid a tight loop. + requeueAfter = r.GracePeriod + } + return ctrl.Result{RequeueAfter: requeueAfter}, nil +} + +func (r *MachinePoolLifecycleReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + Named("MachinePoolLifecycle"). + For(&computev1alpha1.MachinePool{}). + Watches( + &coordinationv1.Lease{}, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []ctrl.Request { + return []ctrl.Request{{NamespacedName: client.ObjectKey{Name: obj.GetName()}}} + }), + builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool { + return obj.GetNamespace() == computev1alpha1.NamespaceMachinePoolLease + })), + ). + Complete(r) +} diff --git a/internal/controllers/compute/machinepool_lifecycle_controller_test.go b/internal/controllers/compute/machinepool_lifecycle_controller_test.go new file mode 100644 index 000000000..39ec69181 --- /dev/null +++ b/internal/controllers/compute/machinepool_lifecycle_controller_test.go @@ -0,0 +1,314 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package compute + +import ( + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + . "sigs.k8s.io/controller-runtime/pkg/envtest/komega" + + computev1alpha1 "github.com/ironcore-dev/ironcore/api/compute/v1alpha1" + coordinationv1 "k8s.io/api/coordination/v1" +) + +var _ = Describe("machinepool lifecycle controller", func() { + machinePool := SetupMachinePool() + + Context("when neither lease nor ready condition see progress within the grace period", func() { + It("should set the ready condition to Unknown (Lease w/o RenewTime) and update Unknown only once", func(ctx SpecContext) { + By("creating a lease for the machine pool without RenewTime") + lease := &coordinationv1.Lease{ + ObjectMeta: metav1.ObjectMeta{ + Name: machinePool.Name, + Namespace: "ironcore-machinepool-lease", + }, + Spec: coordinationv1.LeaseSpec{ + HolderIdentity: ptr.To(machinePool.Name), + LeaseDurationSeconds: ptr.To(int32(3600)), + }, + } + Expect(k8sClient.Create(ctx, lease)).To(Succeed(), "failed to create lease") + DeferCleanup(func(ctx SpecContext) { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, lease))).To(Succeed()) + }) + + By("checking that the MachinePool Ready condition is set to Unknown") + Eventually(ctx, Object(machinePool)).WithTimeout(2 * machinePoolLifecycleGracePeriod).Should( + HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionUnknown), + ))), + ) + + By("verifying the Unknown status is only set once (no further status patches)") + resourceVersion := machinePool.ResourceVersion + + Consistently(ctx, Object(machinePool)).WithTimeout(3 * machinePoolLifecycleGracePeriod).Should( + HaveField("ResourceVersion", Equal(resourceVersion)), + ) + }) + + It("should set the ready condition to Unknown (Lease with RenewTime)", func(ctx SpecContext) { + By("creating a lease for the machine pool") + lease := &coordinationv1.Lease{ + ObjectMeta: metav1.ObjectMeta{ + Name: machinePool.Name, + Namespace: "ironcore-machinepool-lease", + }, + Spec: coordinationv1.LeaseSpec{ + HolderIdentity: ptr.To(machinePool.Name), + LeaseDurationSeconds: ptr.To(int32(3600)), + RenewTime: ptr.To(metav1.NowMicro()), + }, + } + Expect(k8sClient.Create(ctx, lease)).To(Succeed(), "failed to create lease") + DeferCleanup(func(ctx SpecContext) { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, lease))).To(Succeed()) + }) + + By("checking that the MachinePool Ready condition is set to Unknown") + Eventually(ctx, Object(machinePool)).Should( + HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionUnknown), + ))), + ) + }) + + It("should set the ready condition to Unknown (No lease)", func(ctx SpecContext) { + By("checking that the MachinePool Ready condition is set to Unknown") + Eventually(ctx, Object(machinePool)).Should( + HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionUnknown), + ))), + ) + }) + }) + + Context("when the lease is renewed frequently", func() { + It("should not set the ready condition to Unknown when the lease is regularly renewed", func(ctx SpecContext) { + By("creating a lease for the machine pool with a current renewTime") + lease := &coordinationv1.Lease{ + ObjectMeta: metav1.ObjectMeta{ + Name: machinePool.Name, + Namespace: "ironcore-machinepool-lease", + }, + Spec: coordinationv1.LeaseSpec{ + HolderIdentity: ptr.To(machinePool.Name), + LeaseDurationSeconds: ptr.To(int32(3600)), + RenewTime: ptr.To(metav1.NowMicro()), + }, + } + Expect(k8sClient.Create(ctx, lease)).To(Succeed()) + DeferCleanup(func(ctx SpecContext) { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, lease))).To(Succeed()) + }) + + By("continuously renewing the lease well within the grace period") + stop := startLeaseRenewer(lease) + DeferCleanup(stop) + + By("verifying the ready condition never becomes Unknown over 15 seconds") + Consistently(ctx, Object(machinePool)).WithTimeout(3 * machinePoolLifecycleGracePeriod).Should( + Not(HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionUnknown), + )))), + ) + }) + }) + + Context("when the ready condition progresses", func() { + DescribeTable("should not flip a freshly-progressing ready condition to Unknown", + func(ctx SpecContext, initialStatus corev1.ConditionStatus, nextStatus corev1.ConditionStatus) { + By("setting the initial ready condition on the machine pool") + patchReadyCondition(ctx, machinePool, initialStatus, "Initial", "initial state") + + By("continuously refreshing the ready condition well within the grace period") + stop := startReadyConditionRenewer(machinePool, nextStatus) + DeferCleanup(stop) + + By("verifying the ready condition never becomes Unknown") + Consistently(ctx, Object(machinePool)).WithTimeout(3 * machinePoolLifecycleGracePeriod).Should( + Not(HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionUnknown), + )))), + ) + }, + Entry("True remains True", corev1.ConditionTrue, corev1.ConditionTrue), + Entry("False remains False", corev1.ConditionFalse, corev1.ConditionFalse), + Entry("False progressing to True", corev1.ConditionFalse, corev1.ConditionTrue), + Entry("True progressing to False", corev1.ConditionTrue, corev1.ConditionFalse), + ) + }) + + Context("when only one health signal stays fresh", func() { + It("should keep the ready condition healthy when the lease is stale but the ready condition is refreshed", func(ctx SpecContext) { + By("creating a lease with a stale RenewTime") + lease := &coordinationv1.Lease{ + ObjectMeta: metav1.ObjectMeta{ + Name: machinePool.Name, + Namespace: "ironcore-machinepool-lease", + }, + Spec: coordinationv1.LeaseSpec{ + HolderIdentity: ptr.To(machinePool.Name), + LeaseDurationSeconds: ptr.To(int32(3600)), + RenewTime: ptr.To(metav1.NewMicroTime(time.Now().Add(-time.Hour))), + }, + } + Expect(k8sClient.Create(ctx, lease)).To(Succeed()) + DeferCleanup(func(ctx SpecContext) { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, lease))).To(Succeed()) + }) + + By("continuously refreshing the ready condition") + patchReadyCondition(ctx, machinePool, corev1.ConditionTrue, "Healthy", "machinepoollet healthy") + stop := startReadyConditionRenewer(machinePool, corev1.ConditionTrue) + DeferCleanup(stop) + + By("verifying the ready condition never becomes Unknown") + Consistently(ctx, Object(machinePool)).WithTimeout(3 * machinePoolLifecycleGracePeriod).Should( + Not(HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionUnknown), + )))), + ) + }) + + It("should keep the ready condition healthy when the ready condition is stale but the lease is renewed", func(ctx SpecContext) { + By("seeding a stale ready condition on the machine pool") + patchReadyCondition(ctx, machinePool, corev1.ConditionTrue, "Healthy", "machinepoollet healthy") + + By("creating a lease for the machine pool") + lease := &coordinationv1.Lease{ + ObjectMeta: metav1.ObjectMeta{ + Name: machinePool.Name, + Namespace: "ironcore-machinepool-lease", + }, + Spec: coordinationv1.LeaseSpec{ + HolderIdentity: ptr.To(machinePool.Name), + LeaseDurationSeconds: ptr.To(int32(3600)), + RenewTime: ptr.To(metav1.NowMicro()), + }, + } + Expect(k8sClient.Create(ctx, lease)).To(Succeed()) + DeferCleanup(func(ctx SpecContext) { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, lease))).To(Succeed()) + }) + + By("continuously renewing the lease, while the ready condition stays untouched") + stop := startLeaseRenewer(lease) + DeferCleanup(stop) + + By("verifying the ready condition never becomes Unknown") + Consistently(ctx, Object(machinePool)).WithTimeout(3 * machinePoolLifecycleGracePeriod).Should( + Not(HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionUnknown), + )))), + ) + }) + }) + + Context("when a fresh signal arrives after the controller marked the pool Unknown", func() { + It("should stop patching the status once the machinepoollet posts a fresh ready condition", func(ctx SpecContext) { + By("waiting for the controller to set the ready condition to Unknown (no lease, no progress)") + Eventually(ctx, Object(machinePool)).WithTimeout(2 * machinePoolLifecycleGracePeriod).Should( + HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionUnknown), + ))), + ) + + By("posting a fresh ready=True condition as the machinepoollet would") + patchReadyCondition(ctx, machinePool, corev1.ConditionTrue, "Healthy", "machinepoollet recovered") + + By("verifying the controller does not flip the fresh ready condition back to Unknown within the grace period") + Consistently(ctx, Object(machinePool)).WithTimeout(machinePoolLifecycleGracePeriod / 2).Should( + Not(HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionUnknown), + )))), + ) + }) + }) +}) + +func patchReadyCondition(ctx SpecContext, pool *computev1alpha1.MachinePool, status corev1.ConditionStatus, reason, message string) { + GinkgoHelper() + Eventually(ctx, UpdateStatus(pool, func() { + pool.Status.Conditions = computev1alpha1.SetMachinePoolCondition(pool.Status.Conditions, computev1alpha1.MachinePoolCondition{ + Type: computev1alpha1.MachinePoolReady, + Status: status, + Reason: reason, + Message: message, + }) + })).Should(Succeed()) +} + +func startReadyConditionRenewer(pool *computev1alpha1.MachinePool, status corev1.ConditionStatus) func(SpecContext) { + stopCh := make(chan struct{}) + done := make(chan struct{}) + go func() { + defer close(done) + ticker := time.NewTicker(50 * time.Millisecond) + defer ticker.Stop() + counter := 0 + for { + select { + case <-stopCh: + return + case <-ticker.C: + counter++ + c := counter + _ = UpdateStatus(pool, func() { + pool.Status.Conditions = computev1alpha1.SetMachinePoolCondition(pool.Status.Conditions, computev1alpha1.MachinePoolCondition{ + Type: computev1alpha1.MachinePoolReady, + Status: status, + Reason: "MachinePoolReadyChanged", + Message: fmt.Sprintf("machinepool ready changed: %s %d", status, c), + ObservedGeneration: int64(c), + }) + })() + } + } + }() + return func(_ SpecContext) { + close(stopCh) + <-done + } +} + +func startLeaseRenewer(lease *coordinationv1.Lease) func(SpecContext) { + stopCh := make(chan struct{}) + done := make(chan struct{}) + go func() { + defer close(done) + ticker := time.NewTicker(50 * time.Millisecond) + defer ticker.Stop() + for { + select { + case <-stopCh: + return + case <-ticker.C: + _ = Update(lease, func() { + lease.Spec.RenewTime = ptr.To(metav1.NowMicro()) + })() + } + } + }() + return func(_ SpecContext) { + close(stopCh) + <-done + } +} diff --git a/internal/controllers/compute/machinepool_lifecycle_controller_unit_test.go b/internal/controllers/compute/machinepool_lifecycle_controller_unit_test.go new file mode 100644 index 000000000..39d0f9863 --- /dev/null +++ b/internal/controllers/compute/machinepool_lifecycle_controller_unit_test.go @@ -0,0 +1,72 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package compute + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + computev1alpha1 "github.com/ironcore-dev/ironcore/api/compute/v1alpha1" +) + +var _ = Describe("MachinePoolLifecycleReconciler healthData cleanup", func() { + It("removes the healthData entry when the MachinePool no longer exists", func() { + const poolName = "deleted-pool" + + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + + r := &MachinePoolLifecycleReconciler{ + Client: fakeClient, + GracePeriod: 30 * time.Second, + } + r.setMachinePoolHealth(poolName, &MachinePoolHealth{ + lastChangeDetectedTime: time.Now(), + }) + Expect(r.getMachinePoolHealth(poolName)).NotTo(BeNil()) + + result, err := r.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: client.ObjectKey{Name: poolName}, + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + Expect(r.getMachinePoolHealth(poolName)).To(BeNil(), "expected healthData entry to be cleaned up after NotFound") + }) + + It("keeps unrelated healthData entries intact when one MachinePool is deleted", func() { + const deletedPool = "gone-pool" + const otherPool = "other-pool" + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme.Scheme). + WithObjects(&computev1alpha1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{Name: otherPool}, + }). + Build() + + r := &MachinePoolLifecycleReconciler{ + Client: fakeClient, + GracePeriod: 30 * time.Second, + } + other := &MachinePoolHealth{lastChangeDetectedTime: time.Now()} + r.setMachinePoolHealth(deletedPool, &MachinePoolHealth{lastChangeDetectedTime: time.Now()}) + r.setMachinePoolHealth(otherPool, other) + + _, err := r.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: client.ObjectKey{Name: deletedPool}, + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(r.getMachinePoolHealth(deletedPool)).To(BeNil()) + Expect(r.getMachinePoolHealth(otherPool)).To(Equal(other)) + }) +}) diff --git a/internal/controllers/compute/suite_test.go b/internal/controllers/compute/suite_test.go index 39155e100..f834524ae 100644 --- a/internal/controllers/compute/suite_test.go +++ b/internal/controllers/compute/suite_test.go @@ -26,6 +26,7 @@ import ( . "github.com/ironcore-dev/ironcore/utils/testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" @@ -46,10 +47,11 @@ import ( // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. const ( - pollingInterval = 50 * time.Millisecond - eventuallyTimeout = 3 * time.Second - consistentlyDuration = 1 * time.Second - apiServiceTimeout = 5 * time.Minute + pollingInterval = 50 * time.Millisecond + eventuallyTimeout = 3 * time.Second + consistentlyDuration = 1 * time.Second + apiServiceTimeout = 5 * time.Minute + machinePoolLifecycleGracePeriod = 500 * time.Millisecond ) var ( @@ -82,7 +84,7 @@ var _ = BeforeSuite(func() { // default path defined in controller-runtime which is /usr/local/kubebuilder/. // Note that you must have the required binaries setup under the bin directory to perform // the tests directly. When we run make test it will be setup and used automatically. - BinaryAssetsDirectory: filepath.Join("..", "..", "bin", "k8s", + BinaryAssetsDirectory: filepath.Join("..", "..", "..", "bin", "k8s", fmt.Sprintf("1.35.0-%s-%s", runtime.GOOS, runtime.GOARCH)), } testEnvExt = &utilsenvtest.EnvironmentExtensions{ @@ -168,6 +170,11 @@ var _ = BeforeSuite(func() { APIReader: k8sManager.GetAPIReader(), }).SetupWithManager(k8sManager)).To(Succeed()) + Expect((&MachinePoolLifecycleReconciler{ + Client: k8sManager.GetClient(), + GracePeriod: machinePoolLifecycleGracePeriod, + }).SetupWithManager(k8sManager)).To(Succeed()) + Expect((&MachineEvictionReconciler{ EventRecorder: &events.FakeRecorder{}, Client: k8sManager.GetClient(), @@ -177,6 +184,13 @@ var _ = BeforeSuite(func() { defer GinkgoRecover() Expect(k8sManager.Start(ctx)).To(Succeed(), "failed to start manager") }() + + leaseNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: computev1alpha1.NamespaceMachinePoolLease, + }, + } + Expect(k8sClient.Create(ctx, leaseNamespace)).To(Succeed(), "failed to create lease namespace") }) func SetupMachineClass() *computev1alpha1.MachineClass { @@ -209,3 +223,13 @@ func SetupVolumeClass() *storagev1alpha1.VolumeClass { } }) } + +func SetupMachinePool() *computev1alpha1.MachinePool { + return SetupObjectStruct[*computev1alpha1.MachinePool](&k8sClient, func(machinePool *computev1alpha1.MachinePool) { + *machinePool = computev1alpha1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-pool-", + }, + } + }) +} diff --git a/poollet/machinepoollet/cmd/machinepoollet/app/app.go b/poollet/machinepoollet/cmd/machinepoollet/app/app.go index f301a0dcf..8f22ed8dc 100644 --- a/poollet/machinepoollet/cmd/machinepoollet/app/app.go +++ b/poollet/machinepoollet/cmd/machinepoollet/app/app.go @@ -36,15 +36,16 @@ import ( "github.com/ironcore-dev/ironcore/utils/client/config" "github.com/ironcore-dev/controller-utils/configutils" + coordinationv1 "k8s.io/api/coordination/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/certwatcher" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/metrics/filters" @@ -97,6 +98,10 @@ type Options struct { DialTimeout time.Duration MachineClassMapperSyncTimeout time.Duration + HeartbeatInterval time.Duration + HeartbeatLeaseDuration time.Duration + HeartbeatStatusTimeout time.Duration + ChannelCapacity int RelistPeriod time.Duration RelistThreshold time.Duration @@ -149,6 +154,10 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) { fs.DurationVar(&o.DialTimeout, "dial-timeout", 1*time.Second, "Timeout for dialing to the machine runtime endpoint.") fs.DurationVar(&o.MachineClassMapperSyncTimeout, "mcm-sync-timeout", 10*time.Second, "Timeout waiting for the machine class mapper to sync.") + fs.DurationVar(&o.HeartbeatInterval, "heartbeat-interval", 10*time.Second, "Interval between machine pool heartbeats.") + fs.DurationVar(&o.HeartbeatLeaseDuration, "heartbeat-lease-duration", 40*time.Second, "leaseDurationSeconds to publish on the machine pool lease.") + fs.DurationVar(&o.HeartbeatStatusTimeout, "heartbeat-status-timeout", 5*time.Second, "Timeout for the IRI Status probe used as the heartbeat readiness check.") + fs.IntVar(&o.ChannelCapacity, "channel-capacity", 1024, "channel capacity for the machine event generator.") fs.DurationVar(&o.RelistPeriod, "relist-period", 5*time.Second, "event channel relisting period.") fs.DurationVar(&o.RelistThreshold, "relist-threshold", 3*time.Minute, "event channel relisting threshold.") @@ -228,6 +237,18 @@ func Run(ctx context.Context, opts Options) error { return fmt.Errorf("error getting port from address: %w", err) } + if opts.HeartbeatInterval <= 0 { + return fmt.Errorf("--heartbeat-interval must be > 0, got %s", opts.HeartbeatInterval) + } + if opts.HeartbeatLeaseDuration <= opts.HeartbeatInterval { + return fmt.Errorf("--heartbeat-lease-duration (%s) must be greater than --heartbeat-interval (%s)", + opts.HeartbeatLeaseDuration, opts.HeartbeatInterval) + } + if opts.HeartbeatStatusTimeout <= 0 || opts.HeartbeatStatusTimeout >= opts.HeartbeatInterval { + return fmt.Errorf("--heartbeat-status-timeout (%s) must be > 0 and less than --heartbeat-interval (%s)", + opts.HeartbeatStatusTimeout, opts.HeartbeatInterval) + } + getter, err := machinepoolletconfig.NewGetter(opts.MachinePoolName) if err != nil { return fmt.Errorf("error creating new getter: %w", err) @@ -345,15 +366,20 @@ func Run(ctx context.Context, opts Options) error { LeaderElectionID: "bfafcebe.ironcore.dev", LeaderElectionNamespace: opts.LeaderElectionNamespace, LeaderElectionConfig: leaderElectionCfg, - Cache: cache.Options{ByObject: map[client.Object]cache.ByObject{}}, - NewCache: func(config *rest.Config, cacheOpts cache.Options) (cache.Cache, error) { - cacheOpts.ByObject[&computev1alpha1.Machine{}] = cache.ByObject{ - Field: fields.OneTermEqualSelector( - computev1alpha1.MachineMachinePoolRefNameField, - opts.MachinePoolName, - ), - } - return cache.New(config, cacheOpts) + Cache: cache.Options{ + ByObject: map[client.Object]cache.ByObject{ + &computev1alpha1.Machine{}: { + Field: fields.OneTermEqualSelector( + computev1alpha1.MachineMachinePoolRefNameField, + opts.MachinePoolName, + ), + }, + &coordinationv1.Lease{}: { + Namespaces: map[string]cache.Config{ + computev1alpha1.NamespaceMachinePoolLease: {}, + }, + }, + }, }, }) if err != nil { @@ -470,6 +496,9 @@ func Run(ctx context.Context, opts Options) error { return fmt.Errorf("error setting up machine annotator reconciler with manager: %w", err) } + readyState := controllers.NewMachinePoolReadyState() + heartbeatEvents := make(chan event.GenericEvent, 1) + if err := (&controllers.MachinePoolReconciler{ Client: mgr.GetClient(), MachinePoolName: opts.MachinePoolName, @@ -478,10 +507,25 @@ func Run(ctx context.Context, opts Options) error { MachineRuntime: machineRuntime, MachineClassMapper: machineClassMapper, TopologyLabels: topologyLabels, + ReadyState: readyState, + HeartbeatEvents: heartbeatEvents, }).SetupWithManager(mgr); err != nil { return fmt.Errorf("error setting up machine pool reconciler with manager: %w", err) } + if err := mgr.Add(controllers.NewMachinePoolHeartbeat( + mgr.GetClient(), + opts.MachinePoolName, + machineRuntime, + readyState, + heartbeatEvents, + opts.HeartbeatInterval, + opts.HeartbeatLeaseDuration, + opts.HeartbeatStatusTimeout, + )); err != nil { + return fmt.Errorf("error adding machine pool heartbeat: %w", err) + } + if err := (&controllers.MachinePoolAnnotatorReconciler{ Client: mgr.GetClient(), MachinePoolName: opts.MachinePoolName, diff --git a/poollet/machinepoollet/controllers/controllers_suite_test.go b/poollet/machinepoollet/controllers/controllers_suite_test.go index c27a63dee..c54ffbb26 100644 --- a/poollet/machinepoollet/controllers/controllers_suite_test.go +++ b/poollet/machinepoollet/controllers/controllers_suite_test.go @@ -146,6 +146,13 @@ var _ = BeforeSuite(func() { Expect(ctrlMgr.Start()).To(Succeed()) DeferCleanup(ctrlMgr.Stop) + + leaseNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: computev1alpha1.NamespaceMachinePoolLease, + }, + } + Expect(k8sClient.Create(context.TODO(), leaseNamespace)).To(Succeed(), "failed to create lease namespace") }) func SetupTest() (*corev1.Namespace, *computev1alpha1.MachinePool, *computev1alpha1.MachineClass, *machine.FakeRuntimeService) { diff --git a/poollet/machinepoollet/controllers/machinepool_controller.go b/poollet/machinepoollet/controllers/machinepool_controller.go index b6603b050..9627a0f9a 100644 --- a/poollet/machinepoollet/controllers/machinepool_controller.go +++ b/poollet/machinepoollet/controllers/machinepool_controller.go @@ -24,8 +24,10 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/source" ) type MachinePoolReconciler struct { @@ -42,6 +44,18 @@ type MachinePoolReconciler struct { MachineClassMapper mcm.MachineClassMapper TopologyLabels map[commonv1alpha1.TopologyLabel]string + + // ReadyState carries the latest IRI Status probe result published by the + // MachinePoolHeartbeat. When non-nil and populated, the reconciler + // reflects it into Status.Conditions[Ready] as part of its single status + // patch. The reconciler is the only poollet-side writer of that + // condition. May be nil; in that case the condition is left untouched. + ReadyState *MachinePoolReadyState + + // HeartbeatEvents is an optional channel that the heartbeat uses to + // nudge this reconciler when ReadyState changes. SetupWithManager wires + // it as a source.Channel. May be nil. + HeartbeatEvents <-chan event.GenericEvent } //+kubebuilder:rbac:groups=compute.ironcore.dev,resources=machinepools,verbs=get;list;watch;update;patch @@ -135,6 +149,8 @@ func (r *MachinePoolReconciler) updateStatus(ctx context.Context, log logr.Logge machinePool.Status.Allocatable = allocatable machinePool.Status.DaemonEndpoints.MachinepoolletEndpoint.Port = r.Port + r.applyReadyCondition(machinePool) + if err := r.Status().Patch(ctx, machinePool, client.MergeFrom(base)); err != nil { return fmt.Errorf("error patching machine pool status: %w", err) } @@ -142,6 +158,30 @@ func (r *MachinePoolReconciler) updateStatus(ctx context.Context, log logr.Logge return nil } +// applyReadyCondition updates machinePool.Status.Conditions[Ready] from the +// latest IRI Status probe result published by the heartbeat. It is a no-op +// when no ReadyState is configured or the heartbeat has not produced a +// result yet, so the existing condition (e.g. a Ready=Unknown set by the +// control-plane lifecycle controller) is preserved until the poollet has an +// opinion of its own. +func (r *MachinePoolReconciler) applyReadyCondition(machinePool *computev1alpha1.MachinePool) { + if r.ReadyState == nil { + return + } + probeErr, hasResult := r.ReadyState.Get() + if !hasResult { + return + } + + desired := ComputeReadyCondition(machinePool.Generation, probeErr) + existing := computev1alpha1.FindMachinePoolCondition(machinePool.Status.Conditions, computev1alpha1.MachinePoolReady) + if !ReadyConditionsDiffer(existing, desired) { + return + } + + machinePool.Status.Conditions = computev1alpha1.SetMachinePoolCondition(machinePool.Status.Conditions, desired) +} + func (r *MachinePoolReconciler) reconcile(ctx context.Context, log logr.Logger, machinePool *computev1alpha1.MachinePool) (ctrl.Result, error) { log.V(1).Info("Reconcile") @@ -192,7 +232,7 @@ func (r *MachinePoolReconciler) enforceOriginalTopologyLabels(ctx context.Contex } func (r *MachinePoolReconciler) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). + b := ctrl.NewControllerManagedBy(mgr). For( &computev1alpha1.MachinePool{}, builder.WithPredicates( @@ -215,6 +255,18 @@ func (r *MachinePoolReconciler) SetupWithManager(mgr ctrl.Manager) error { builder.WithPredicates( MachineRunsInMachinePoolPredicate(r.MachinePoolName), ), - ). - Complete(r) + ) + + if r.HeartbeatEvents != nil { + b = b.WatchesRawSource( + source.Channel( + r.HeartbeatEvents, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []ctrl.Request { + return []ctrl.Request{{NamespacedName: client.ObjectKey{Name: r.MachinePoolName}}} + }), + ), + ) + } + + return b.Complete(r) } diff --git a/poollet/machinepoollet/controllers/machinepool_heartbeat.go b/poollet/machinepoollet/controllers/machinepool_heartbeat.go new file mode 100644 index 000000000..dfd704c85 --- /dev/null +++ b/poollet/machinepoollet/controllers/machinepool_heartbeat.go @@ -0,0 +1,290 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package controllers + +import ( + "context" + "fmt" + "sync" + "time" + + "github.com/go-logr/logr" + "github.com/google/uuid" + computev1alpha1 "github.com/ironcore-dev/ironcore/api/compute/v1alpha1" + "github.com/ironcore-dev/ironcore/iri/apis/machine" + iri "github.com/ironcore-dev/ironcore/iri/apis/machine/v1alpha1" + coordinationv1 "k8s.io/api/coordination/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" +) + +const ( + readyReasonHeartbeatReceived = "HeartbeatReceived" + readyReasonRuntimeUnreachable = "RuntimeUnreachable" + + readyMessageHeartbeatReceived = "machine runtime status probe succeeded" +) + +// ComputeReadyCondition returns the MachinePoolCondition that reflects the +// result of the most recent IRI Status probe for the given pool generation. +func ComputeReadyCondition(generation int64, probeErr error) computev1alpha1.MachinePoolCondition { + if probeErr != nil { + return computev1alpha1.MachinePoolCondition{ + Type: computev1alpha1.MachinePoolReady, + Status: corev1.ConditionFalse, + Reason: readyReasonRuntimeUnreachable, + Message: probeErr.Error(), + ObservedGeneration: generation, + } + } + return computev1alpha1.MachinePoolCondition{ + Type: computev1alpha1.MachinePoolReady, + Status: corev1.ConditionTrue, + Reason: readyReasonHeartbeatReceived, + Message: readyMessageHeartbeatReceived, + ObservedGeneration: generation, + } +} + +// ReadyConditionsDiffer reports whether existing differs from desired in any +// field that justifies a status patch. LastUpdateTime and LastTransitionTime +// are intentionally ignored — they are bookkeeping that should only advance +// when something else also advances. A nil existing always differs. +func ReadyConditionsDiffer(existing *computev1alpha1.MachinePoolCondition, desired computev1alpha1.MachinePoolCondition) bool { + if existing == nil { + return true + } + return existing.Status != desired.Status || + existing.Reason != desired.Reason || + existing.Message != desired.Message || + existing.ObservedGeneration != desired.ObservedGeneration +} + +// MachinePoolReadyState is a thread-safe holder of the most recent IRI +// Status probe result. The MachinePoolHeartbeat publishes results into it; +// the MachinePoolReconciler reads them when computing the Ready condition. +// +// hasResult is false until the first tick has run, which lets the +// reconciler avoid clobbering whatever value is currently on the pool +// (e.g. a Ready=Unknown set by the lifecycle controller) before the +// poollet has any opinion of its own. +type MachinePoolReadyState struct { + mu sync.RWMutex + hasResult bool + probeErr error +} + +// NewMachinePoolReadyState returns a fresh, empty state. +func NewMachinePoolReadyState() *MachinePoolReadyState { + return &MachinePoolReadyState{} +} + +// Set stores the latest probe result and reports whether it differs from the +// previously stored value. The very first call always reports changed=true. +// Two errors are considered equal if their Error() strings match — that is +// what feeds into the condition's Message field. +func (s *MachinePoolReadyState) Set(probeErr error) (changed bool) { + s.mu.Lock() + defer s.mu.Unlock() + + if !s.hasResult { + s.hasResult = true + s.probeErr = probeErr + return true + } + + if errString(s.probeErr) == errString(probeErr) { + return false + } + + s.probeErr = probeErr + return true +} + +// Get returns the latest probe result and whether any tick has stored one. +func (s *MachinePoolReadyState) Get() (probeErr error, hasResult bool) { + s.mu.RLock() + defer s.mu.RUnlock() + return s.probeErr, s.hasResult +} + +func errString(err error) string { + if err == nil { + return "" + } + return err.Error() +} + +// MachinePoolHeartbeat is a manager.Runnable that periodically renews the +// pool's Lease in NamespaceMachinePoolLease and publishes the latest IRI +// Status probe result into a shared MachinePoolReadyState. When the probe +// result changes (or on the first tick), it sends a generic event on Events +// so the MachinePoolReconciler — the actual writer of the pool's Status — +// reconciles the new condition. +type MachinePoolHeartbeat struct { + Client client.Client + MachinePoolName string + MachineRuntime machine.RuntimeService + + // ReadyState receives the latest probe result on every tick. + ReadyState *MachinePoolReadyState + + // Events is owned by the caller. The heartbeat sends a single + // GenericEvent for the pool whenever the probe result changes, so the + // MachinePoolReconciler picks up the change without waiting for an + // unrelated reconcile trigger. May be nil in tests where no reconciler + // consumes the events. + Events chan<- event.GenericEvent + + HeartbeatInterval time.Duration + LeaseDuration time.Duration + StatusProbeTimeout time.Duration + + holderIdentity string +} + +// NewMachinePoolHeartbeat constructs a heartbeat runnable. The holderIdentity +// is fixed for the process lifetime: _. +func NewMachinePoolHeartbeat( + c client.Client, + machinePoolName string, + machineRuntime machine.RuntimeService, + readyState *MachinePoolReadyState, + events chan<- event.GenericEvent, + heartbeatInterval, leaseDuration, statusProbeTimeout time.Duration, +) *MachinePoolHeartbeat { + return &MachinePoolHeartbeat{ + Client: c, + MachinePoolName: machinePoolName, + MachineRuntime: machineRuntime, + ReadyState: readyState, + Events: events, + HeartbeatInterval: heartbeatInterval, + LeaseDuration: leaseDuration, + StatusProbeTimeout: statusProbeTimeout, + holderIdentity: fmt.Sprintf("%s_%s", machinePoolName, uuid.NewString()), + } +} + +// Start runs the heartbeat loop until ctx is canceled. It satisfies +// sigs.k8s.io/controller-runtime/pkg/manager.Runnable. +func (h *MachinePoolHeartbeat) Start(ctx context.Context) error { + log := ctrl.LoggerFrom(ctx).WithName("machinepool-heartbeat") + log.Info("Starting machine pool heartbeat", + "interval", h.HeartbeatInterval, + "leaseDuration", h.LeaseDuration, + "holderIdentity", h.holderIdentity, + ) + + h.tick(ctx, log) + + ticker := time.NewTicker(h.HeartbeatInterval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + log.Info("Stopping machine pool heartbeat") + return nil + case <-ticker.C: + h.tick(ctx, log) + } + } +} + +func (h *MachinePoolHeartbeat) tick(ctx context.Context, log logr.Logger) { + probeCtx, cancel := context.WithTimeout(ctx, h.StatusProbeTimeout) + _, statusErr := h.MachineRuntime.Status(probeCtx, &iri.StatusRequest{}) + cancel() + + if err := h.reconcileLease(ctx); err != nil && ctx.Err() == nil { + log.Error(err, "Failed to reconcile machine pool lease") + } + + h.publishReadyState(ctx, log, statusErr) +} + +// publishReadyState stores the latest probe result and, if it differs from +// the previous one, kicks the MachinePoolReconciler via the event channel so +// the new Ready condition is reflected on the pool without waiting for an +// unrelated reconcile trigger. +func (h *MachinePoolHeartbeat) publishReadyState(ctx context.Context, log logr.Logger, statusErr error) { + if h.ReadyState == nil { + return + } + + changed := h.ReadyState.Set(statusErr) + if !changed || h.Events == nil { + return + } + + evt := event.GenericEvent{ + Object: &computev1alpha1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{Name: h.MachinePoolName}, + }, + } + select { + case h.Events <- evt: + case <-ctx.Done(): + default: + // The reconciler is already enqueued or the channel has no + // reader yet. Either way, the next reconcile will pick up the + // updated state from ReadyState — dropping is safe. + log.V(1).Info("Dropping machine pool ready-state event; channel is full or has no reader") + } +} + +func (h *MachinePoolHeartbeat) reconcileLease(ctx context.Context) error { + leaseDurationSeconds := int32(h.LeaseDuration.Seconds()) + now := metav1.NewMicroTime(time.Now()) + + lease := &coordinationv1.Lease{} + key := client.ObjectKey{Namespace: computev1alpha1.NamespaceMachinePoolLease, Name: h.MachinePoolName} + if err := h.Client.Get(ctx, key, lease); err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("getting lease: %w", err) + } + newLease := &coordinationv1.Lease{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: key.Namespace, + Name: key.Name, + }, + Spec: coordinationv1.LeaseSpec{ + HolderIdentity: ptr.To(h.holderIdentity), + LeaseDurationSeconds: ptr.To(leaseDurationSeconds), + AcquireTime: ptr.To(now), + RenewTime: ptr.To(now), + }, + } + if err := h.Client.Create(ctx, newLease); err != nil { + return fmt.Errorf("creating lease: %w", err) + } + return nil + } + + base := lease.DeepCopy() + if lease.Spec.HolderIdentity == nil || *lease.Spec.HolderIdentity != h.holderIdentity { + // Take ownership; the previous owner (likely a previous poollet process for + // this pool) is gone or stale. + log := ctrl.LoggerFrom(ctx) + previousHolder := "" + if lease.Spec.HolderIdentity != nil { + previousHolder = *lease.Spec.HolderIdentity + } + log.Info("Taking ownership of stale machine pool lease", "previousHolder", previousHolder, "newHolder", h.holderIdentity) + lease.Spec.HolderIdentity = ptr.To(h.holderIdentity) + lease.Spec.AcquireTime = ptr.To(now) + } + lease.Spec.LeaseDurationSeconds = ptr.To(leaseDurationSeconds) + lease.Spec.RenewTime = ptr.To(now) + + if err := h.Client.Patch(ctx, lease, client.MergeFrom(base)); err != nil { + return fmt.Errorf("patching lease: %w", err) + } + return nil +} diff --git a/poollet/machinepoollet/controllers/machinepool_heartbeat_test.go b/poollet/machinepoollet/controllers/machinepool_heartbeat_test.go new file mode 100644 index 000000000..f9cf43c5c --- /dev/null +++ b/poollet/machinepoollet/controllers/machinepool_heartbeat_test.go @@ -0,0 +1,212 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package controllers_test + +import ( + "context" + "errors" + "sync/atomic" + "time" + + commonv1alpha1 "github.com/ironcore-dev/ironcore/api/common/v1alpha1" + computev1alpha1 "github.com/ironcore-dev/ironcore/api/compute/v1alpha1" + computeclient "github.com/ironcore-dev/ironcore/internal/client/compute" + "github.com/ironcore-dev/ironcore/iri/apis/machine" + iri "github.com/ironcore-dev/ironcore/iri/apis/machine/v1alpha1" + fakemachine "github.com/ironcore-dev/ironcore/iri/testing/machine" + "github.com/ironcore-dev/ironcore/poollet/machinepoollet/controllers" + "github.com/ironcore-dev/ironcore/poollet/machinepoollet/mcm" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" + coordinationv1 "k8s.io/api/coordination/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + ctrlconfig "sigs.k8s.io/controller-runtime/pkg/config" + . "sigs.k8s.io/controller-runtime/pkg/envtest/komega" + "sigs.k8s.io/controller-runtime/pkg/event" + metricserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" +) + +// errorInjectingRuntime wraps a real fake runtime and lets a test toggle +// errors on the Status method only. All other calls pass through. +type errorInjectingRuntime struct { + machine.RuntimeService + statusErr atomic.Pointer[error] +} + +func (r *errorInjectingRuntime) Status(ctx context.Context, req *iri.StatusRequest) (*iri.StatusResponse, error) { + if errPtr := r.statusErr.Load(); errPtr != nil && *errPtr != nil { + return nil, *errPtr + } + return r.RuntimeService.Status(ctx, req) +} + +func (r *errorInjectingRuntime) setStatusErr(err error) { + if err == nil { + r.statusErr.Store(nil) + return + } + r.statusErr.Store(&err) +} + +var _ = Describe("MachinePoolHeartbeat", func() { + var ( + mp *computev1alpha1.MachinePool + runner *errorInjectingRuntime + ) + + BeforeEach(func(ctx SpecContext) { + By("creating a pool for this spec") + mp = &computev1alpha1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "heartbeat-mp-"}, + } + Expect(k8sClient.Create(ctx, mp)).To(Succeed()) + DeferCleanup(k8sClient.Delete, mp) + + fake := fakemachine.NewFakeRuntimeService() + runner = &errorInjectingRuntime{RuntimeService: fake} + + // Stand up a manager just for this spec — keep it isolated from the + // shared SetupTest manager so we control which runnables are added. + mgr, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme.Scheme, + Metrics: metricserver.Options{BindAddress: "0"}, + Controller: ctrlconfig.Controller{SkipNameValidation: ptr.To(true)}, + }) + Expect(err).NotTo(HaveOccurred()) + + // The reconciler needs the same indexer the production wiring sets up. + Expect(computeclient.SetupMachineSpecMachinePoolRefNameFieldIndexer(ctx, mgr.GetFieldIndexer())).To(Succeed()) + + machineClassMapper := mcm.NewGeneric(runner, mcm.GenericOptions{ + RelistPeriod: 2 * time.Second, + }) + Expect(mgr.Add(machineClassMapper)).To(Succeed()) + + readyState := controllers.NewMachinePoolReadyState() + heartbeatEvents := make(chan event.GenericEvent, 1) + + Expect((&controllers.MachinePoolReconciler{ + Client: mgr.GetClient(), + MachinePoolName: mp.Name, + MachineRuntime: runner, + MachineClassMapper: machineClassMapper, + TopologyLabels: map[commonv1alpha1.TopologyLabel]string{}, + ReadyState: readyState, + HeartbeatEvents: heartbeatEvents, + }).SetupWithManager(mgr)).To(Succeed()) + + hb := controllers.NewMachinePoolHeartbeat( + mgr.GetClient(), mp.Name, runner, + readyState, heartbeatEvents, + 500*time.Millisecond, // interval + 3*time.Second, // lease duration + 200*time.Millisecond, // status probe timeout + ) + Expect(mgr.Add(hb)).To(Succeed()) + + mgrCtx, cancel := context.WithCancel(context.Background()) + DeferCleanup(cancel) + go func() { + defer GinkgoRecover() + Expect(mgr.Start(mgrCtx)).To(Succeed()) + }() + }) + + It("creates and renews the lease and sets Ready=True on success", func() { + lease := &coordinationv1.Lease{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: computev1alpha1.NamespaceMachinePoolLease, + Name: mp.Name, + }, + } + + By("waiting for the lease to be created with the expected shape") + Eventually(Object(lease)).Should(SatisfyAll( + HaveField("Spec.HolderIdentity", Not(BeNil())), + HaveField("Spec.HolderIdentity", PointTo(HavePrefix(mp.Name+"_"))), + HaveField("Spec.LeaseDurationSeconds", Not(BeNil())), + HaveField("Spec.LeaseDurationSeconds", PointTo(Equal(int32(3)))), + HaveField("Spec.RenewTime", Not(BeNil())), + )) + + pool := &computev1alpha1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{Name: mp.Name}, + } + + By("waiting for the Ready condition to be set to True") + Eventually(Object(pool)).Should( + HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionTrue), + HaveField("Reason", Equal("HeartbeatReceived")), + ))), + ) + + By("capturing the current renewTime") + var first time.Time + Eventually(func(g Gomega) { + g.Expect(Object(lease)()).To(HaveField("Spec.RenewTime", Not(BeNil()))) + first = lease.Spec.RenewTime.Time + }).Should(Succeed()) + + By("observing that the lease gets renewed") + Eventually(func(g Gomega) { + g.Expect(Object(lease)()).To(HaveField("Spec.RenewTime", Not(BeNil()))) + g.Expect(lease.Spec.RenewTime.Time.After(first)).To(BeTrue(), "lease should have been renewed") + }).Should(Succeed()) + }) + + It("does not bump the pool's resourceVersion on no-op heartbeats", func() { + pool := &computev1alpha1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{Name: mp.Name}, + } + + By("waiting for the first Ready=True patch and capturing the resourceVersion") + Eventually(Object(pool)).Should( + HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionTrue), + ))), + ) + initialRV := pool.ResourceVersion + + By("verifying the resourceVersion does not change across subsequent ticks") + Consistently(Object(pool)).Should( + HaveField("ResourceVersion", Equal(initialRV)), + ) + }) + + It("flips Ready to False when the runtime probe errors", func() { + pool := &computev1alpha1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{Name: mp.Name}, + } + + By("waiting for Ready=True before injecting failures") + Eventually(Object(pool)).Should( + HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionTrue), + ))), + ) + + By("injecting a Status error into the fake runtime") + runner.setStatusErr(errors.New("simulated runtime down")) + + By("observing Ready flip to False with RuntimeUnreachable") + Eventually(Object(pool)).Should( + HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", computev1alpha1.MachinePoolReady), + HaveField("Status", corev1.ConditionFalse), + HaveField("Reason", Equal("RuntimeUnreachable")), + HaveField("Message", ContainSubstring("simulated runtime down")), + ))), + ) + }) +}) diff --git a/poollet/machinepoollet/controllers/machinepool_heartbeat_unit_test.go b/poollet/machinepoollet/controllers/machinepool_heartbeat_unit_test.go new file mode 100644 index 000000000..5010fa5d5 --- /dev/null +++ b/poollet/machinepoollet/controllers/machinepool_heartbeat_unit_test.go @@ -0,0 +1,111 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package controllers_test + +import ( + "errors" + + computev1alpha1 "github.com/ironcore-dev/ironcore/api/compute/v1alpha1" + "github.com/ironcore-dev/ironcore/poollet/machinepoollet/controllers" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" +) + +var _ = Describe("ComputeReadyCondition", func() { + It("returns Ready=True with HeartbeatReceived when the probe succeeds", func() { + got := controllers.ComputeReadyCondition(int64(7), nil) + Expect(got.Type).To(Equal(computev1alpha1.MachinePoolReady)) + Expect(got.Status).To(Equal(corev1.ConditionTrue)) + Expect(got.Reason).To(Equal("HeartbeatReceived")) + Expect(got.ObservedGeneration).To(Equal(int64(7))) + }) + + It("returns Ready=False with RuntimeUnreachable when the probe errors", func() { + got := controllers.ComputeReadyCondition(int64(3), errors.New("boom")) + Expect(got.Status).To(Equal(corev1.ConditionFalse)) + Expect(got.Reason).To(Equal("RuntimeUnreachable")) + Expect(got.Message).To(Equal("boom")) + Expect(got.ObservedGeneration).To(Equal(int64(3))) + }) +}) + +var _ = Describe("ReadyConditionsDiffer", func() { + base := computev1alpha1.MachinePoolCondition{ + Type: computev1alpha1.MachinePoolReady, + Status: corev1.ConditionTrue, + Reason: "HeartbeatReceived", + Message: "ok", + ObservedGeneration: 5, + } + + It("treats a nil existing as a diff", func() { + desired := computev1alpha1.MachinePoolCondition{ + Type: computev1alpha1.MachinePoolReady, + Status: corev1.ConditionTrue, + } + Expect(controllers.ReadyConditionsDiffer(nil, desired)).To(BeTrue()) + }) + + It("returns false for identical conditions", func() { + Expect(controllers.ReadyConditionsDiffer(&base, base)).To(BeFalse()) + }) + + It("ignores LastUpdateTime and LastTransitionTime", func() { + desired := base // value copy; timestamps stay zero + Expect(controllers.ReadyConditionsDiffer(&base, desired)).To(BeFalse()) + }) + + DescribeTable("reports a diff when a meaningful field changes", + func(modify func(*computev1alpha1.MachinePoolCondition)) { + desired := base + modify(&desired) + Expect(controllers.ReadyConditionsDiffer(&base, desired)).To(BeTrue()) + }, + Entry("status", func(c *computev1alpha1.MachinePoolCondition) { c.Status = corev1.ConditionFalse }), + Entry("reason", func(c *computev1alpha1.MachinePoolCondition) { c.Reason = "Other" }), + Entry("message", func(c *computev1alpha1.MachinePoolCondition) { c.Message = "different" }), + Entry("observedGeneration", func(c *computev1alpha1.MachinePoolCondition) { c.ObservedGeneration = 6 }), + ) +}) + +var _ = Describe("MachinePoolReadyState", func() { + It("reports hasResult=false until the first Set", func() { + s := controllers.NewMachinePoolReadyState() + err, hasResult := s.Get() + Expect(hasResult).To(BeFalse()) + Expect(err).ToNot(HaveOccurred()) + }) + + It("reports changed=true on the first Set, even with a nil error", func() { + s := controllers.NewMachinePoolReadyState() + Expect(s.Set(nil)).To(BeTrue()) + err, hasResult := s.Get() + Expect(hasResult).To(BeTrue()) + Expect(err).ToNot(HaveOccurred()) + }) + + It("reports changed=false when the error string is unchanged", func() { + s := controllers.NewMachinePoolReadyState() + Expect(s.Set(nil)).To(BeTrue()) + Expect(s.Set(nil)).To(BeFalse()) + + Expect(s.Set(errors.New("boom"))).To(BeTrue()) + Expect(s.Set(errors.New("boom"))).To(BeFalse()) + }) + + It("reports changed=true when the error string changes", func() { + s := controllers.NewMachinePoolReadyState() + Expect(s.Set(errors.New("first"))).To(BeTrue()) + Expect(s.Set(errors.New("second"))).To(BeTrue()) + }) + + It("reports changed=true when transitioning from success to failure and back", func() { + s := controllers.NewMachinePoolReadyState() + Expect(s.Set(nil)).To(BeTrue()) + Expect(s.Set(errors.New("boom"))).To(BeTrue()) + Expect(s.Set(nil)).To(BeTrue()) + }) +}) diff --git a/poollet/machinepoollet/controllers/rbac.go b/poollet/machinepoollet/controllers/rbac.go index 2810703f8..d1345695d 100644 --- a/poollet/machinepoollet/controllers/rbac.go +++ b/poollet/machinepoollet/controllers/rbac.go @@ -13,3 +13,6 @@ package controllers // Rules required for machinepoollet delegated authentication //+kubebuilder:rbac:groups=authentication.k8s.io,resources=tokenreviews,verbs=create //+kubebuilder:rbac:groups=authorization.k8s.io,resources=subjectaccessreviews,verbs=create + +// Rules required for machine pool heartbeat +//+kubebuilder:rbac:groups=coordination.k8s.io,namespace=ironcore-machinepool-lease,resources=leases,verbs=get;list;watch;create;update;patch