diff --git a/config/components/controller_rbac/role.yaml b/config/components/controller_rbac/role.yaml index 935b4b8a..acedcc22 100644 --- a/config/components/controller_rbac/role.yaml +++ b/config/components/controller_rbac/role.yaml @@ -98,6 +98,14 @@ rules: - get - list - watch +- apiGroups: + - networking.datumapis.com + resources: + - networkinterfaces/status + verbs: + - get + - patch + - update - apiGroups: - quota.miloapis.com resources: diff --git a/internal/controller/instance_controller.go b/internal/controller/instance_controller.go index 4dfb8358..e637ef44 100644 --- a/internal/controller/instance_controller.go +++ b/internal/controller/instance_controller.go @@ -250,6 +250,7 @@ type InstanceReconciler struct { // +kubebuilder:rbac:groups=quota.miloapis.com,resources=resourceclaims,verbs=get;list;watch;create;delete // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkinterfaceclaims,verbs=get;list;watch // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkinterfaces,verbs=get;list;watch +// +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkinterfaces/status,verbs=get;update;patch // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get // +kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch @@ -346,6 +347,14 @@ func (r *InstanceReconciler) Reconcile(ctx context.Context, req mcreconcile.Requ readyErr = fmt.Errorf("failed reconciling network interface status: %w", interfacesErr) } + // Publish availability to the interfaces the instance holds while every + // condition is settled in memory. This sits above the quota and status-update + // returns below, because an instance stuck on one of them is precisely an + // instance that should stop receiving traffic. + if err := r.reconcileHolderAvailability(ctx, cl.GetClient(), &instance); err != nil && readyErr == nil { + readyErr = err + } + if statusChanged || readyChanged { if err := cl.GetClient().Status().Update(ctx, &instance); err != nil { if quotaReq > 0 && apierrors.IsConflict(err) { @@ -999,6 +1008,12 @@ func (r *InstanceReconciler) reconcileSuspendedState( } } + // A suspended instance must stop receiving traffic, so the interfaces it + // holds are told before the hub is. + if err := r.reconcileHolderAvailability(ctx, cl, instance); err != nil { + return err + } + // Write suspended status back to the federation hub so the management // plane aggregates the correct per-instance state. return r.writeBackToUpstream(ctx, instance) @@ -1007,6 +1022,12 @@ func (r *InstanceReconciler) reconcileSuspendedState( // reconcileDeletion handles quota-claim cleanup when an Instance is being // deleted. It removes the quota finalizer once the ResourceClaim is gone. func (r *InstanceReconciler) reconcileDeletion(ctx context.Context, cl client.Client, clusterName multicluster.ClusterName, instance *computev1alpha.Instance) error { + // Stop traffic first. Deletion is the drain path that matters — every + // scale-down, rolling update, and redeploy goes through it — and the + // interfaces must be told before the instance's own cleanup runs, not after. + // Best effort: this must never be what keeps an instance in Terminating. + r.drainHolderAvailability(ctx, cl, instance) + if !controllerutil.ContainsFinalizer(instance, instanceQuotaFinalizer) { return nil } diff --git a/internal/controller/networkinterface_holder.go b/internal/controller/networkinterface_holder.go new file mode 100644 index 00000000..d295c382 --- /dev/null +++ b/internal/controller/networkinterface_holder.go @@ -0,0 +1,171 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "context" + "errors" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + computev1alpha "go.datum.net/compute/api/v1alpha" + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" +) + +// networkInterfaceHolderAvailable is the condition compute publishes on every +// NetworkInterface an instance holds, reporting whether the holder considers +// itself able to serve. Networking selects members by it without ever learning +// that the holder is a workload. +// +// It is deliberately not NetworkInterfacePhaseAvailable, which reports that no +// claim holds the interface — the opposite of a healthy holder. +const networkInterfaceHolderAvailable = "HolderAvailable" + +const ( + // holderReasonNotReported covers an interface whose holder has published no + // availability state at all, which is not the same as a holder that reported + // itself unavailable. + holderReasonNotReported = "HolderNotReported" + + // holderReasonUnavailable is the fallback for a holder that reported itself + // unavailable without naming a reason. + holderReasonUnavailable = "HolderUnavailable" + + // holderReasonTerminating covers a holder being deleted. Its own Available + // condition can still read True for the whole deletion window, so deletion is + // judged before it. + holderReasonTerminating = "HolderTerminating" +) + +const ( + msgHolderNotReported = "The instance holding this interface has not reported an availability state" + + msgHolderTerminating = "The instance holding this interface is terminating" +) + +// holderAvailableCondition translates an instance's Available condition into the +// condition networking reads on the interfaces it holds. +// +// Anything other than Available=True yields False, carrying the instance's own +// reason and message: Unknown means nobody has vouched for the instance, and a +// member that cannot be vouched for must drain rather than keep taking traffic. +func holderAvailableCondition(instance *computev1alpha.Instance) metav1.Condition { + condition := metav1.Condition{ + Type: networkInterfaceHolderAvailable, + Status: metav1.ConditionFalse, + Reason: holderReasonNotReported, + Message: msgHolderNotReported, + } + + // A terminating instance is not serving, whatever it last said about itself: + // nothing clears Available on the way out, so it reads True for the whole + // deletion window. Deletion is decided first so a scale-down or a rolling + // update drains the member instead of routing to a process shutting down. + if !instance.DeletionTimestamp.IsZero() { + condition.Reason = holderReasonTerminating + condition.Message = msgHolderTerminating + return condition + } + + available := apimeta.FindStatusCondition(instance.Status.Conditions, computev1alpha.InstanceAvailable) + if available == nil { + return condition + } + + if available.Status == metav1.ConditionTrue { + condition.Status = metav1.ConditionTrue + condition.Reason = computev1alpha.InstanceAvailableReasonAvailable + condition.Message = msgInstanceAvailable + return condition + } + + if available.Reason != "" { + condition.Reason = available.Reason + } else { + condition.Reason = holderReasonUnavailable + } + condition.Message = available.Message + if condition.Message == "" { + condition.Message = fmt.Sprintf("Instance %q is not available", instance.Name) + } + return condition +} + +// reconcileHolderAvailability projects the instance's availability onto every +// NetworkInterface it holds, so networking can decide membership health without +// depending on a fabric that may not be attached. +// +// It runs on every pass rather than only on a transition, so interfaces that +// predate the condition pick it up without their workload being redeployed. +// Writes only when the condition actually moves, so a steady instance produces +// no API traffic. +// +// The condition is derived wholly from the current holder, never merged with +// what is already on the interface. A reclaimPolicy Retain interface outlives +// the instance that held it, so a True left by a previous holder must not be +// inherited by the next one before it is serving. +func (r *InstanceReconciler) reconcileHolderAvailability( + ctx context.Context, + clusterClient client.Client, + instance *computev1alpha.Instance, +) error { + if !r.NetworkingEnabled { + return nil + } + + condition := holderAvailableCondition(instance) + + var errs []error + for _, interfaceStatus := range instance.Status.NetworkInterfaces { + ref := interfaceStatus.NetworkInterfaceRef + if ref == nil || ref.Name == "" { + continue + } + + key := client.ObjectKey{Namespace: instance.Namespace, Name: ref.Name} + var networkInterface networkingv1alpha.NetworkInterface + if err := clusterClient.Get(ctx, key, &networkInterface); err != nil { + if apierrors.IsNotFound(err) { + continue + } + errs = append(errs, fmt.Errorf("get network interface %s: %w", key, err)) + continue + } + + projected := condition + projected.ObservedGeneration = networkInterface.Generation + if !apimeta.SetStatusCondition(&networkInterface.Status.Conditions, projected) { + continue + } + + if err := clusterClient.Status().Update(ctx, &networkInterface); err != nil { + errs = append(errs, fmt.Errorf("update network interface %s status: %w", key, err)) + } + } + + return errors.Join(errs...) +} + +// drainHolderAvailability reports the holder as gone from the interfaces it +// held, without letting that write become a reason the caller cannot proceed. +// +// It is used on the deletion path, where the failure to reach an interface must +// not wedge finalizer removal: an instance that cannot drain must still be able +// to finish deleting, or a transient API error strands it in Terminating +// forever. The interface being gone already is the expected case under a +// reclaimPolicy of Delete and is not a failure at all. +func (r *InstanceReconciler) drainHolderAvailability( + ctx context.Context, + clusterClient client.Client, + instance *computev1alpha.Instance, +) { + if err := r.reconcileHolderAvailability(ctx, clusterClient, instance); err != nil { + log.FromContext(ctx).Error(err, "failed draining network interfaces; deletion continues", + "instance", instance.Name, "namespace", instance.Namespace) + } +} diff --git a/internal/controller/networkinterface_holder_test.go b/internal/controller/networkinterface_holder_test.go new file mode 100644 index 00000000..84437cb9 --- /dev/null +++ b/internal/controller/networkinterface_holder_test.go @@ -0,0 +1,467 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + computev1alpha "go.datum.net/compute/api/v1alpha" + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" +) + +const holderTestInterface = "holder-test-eth0" + +// newHolderTestInstance builds an instance already publishing one bound +// interface, which is the shape every instance reaches once its claim binds. +func newHolderTestInstance(available *metav1.Condition) *computev1alpha.Instance { + instance := &computev1alpha.Instance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "holder-test-instance", + Namespace: claimTestNamespace, + }, + Status: computev1alpha.InstanceStatus{ + NetworkInterfaces: []computev1alpha.InstanceNetworkInterfaceStatus{{ + Name: defaultInterfaceName, + NetworkInterfaceRef: &networkingv1alpha.LocalNetworkInterfaceRef{ + Name: holderTestInterface, + }, + }}, + }, + } + if available != nil { + apimeta.SetStatusCondition(&instance.Status.Conditions, *available) + } + return instance +} + +func newHolderTestInterface() *networkingv1alpha.NetworkInterface { + return &networkingv1alpha.NetworkInterface{ + ObjectMeta: metav1.ObjectMeta{ + Name: holderTestInterface, + Namespace: claimTestNamespace, + Generation: 4, + }, + } +} + +func holderTestClient(objects ...client.Object) client.Client { + return fake.NewClientBuilder(). + WithScheme(newClaimTestScheme()). + WithObjects(objects...). + WithStatusSubresource(&networkingv1alpha.NetworkInterface{}, &computev1alpha.Instance{}). + Build() +} + +func getHolderCondition(t *testing.T, cl client.Client) *metav1.Condition { + t.Helper() + var networkInterface networkingv1alpha.NetworkInterface + require.NoError(t, cl.Get(context.Background(), client.ObjectKey{ + Namespace: claimTestNamespace, + Name: holderTestInterface, + }, &networkInterface)) + return apimeta.FindStatusCondition(networkInterface.Status.Conditions, networkInterfaceHolderAvailable) +} + +// TestHolderAvailableCondition covers the translation from the instance's own +// Available condition, including the states that must not read as healthy. +func TestHolderAvailableCondition(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + available *metav1.Condition + wantStatus metav1.ConditionStatus + wantReason string + wantMessage string + }{ + { + name: "available instance vouches for its interfaces", + available: &metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionTrue, + Reason: computev1alpha.InstanceAvailableReasonAvailable, + }, + wantStatus: metav1.ConditionTrue, + wantReason: computev1alpha.InstanceAvailableReasonAvailable, + wantMessage: msgInstanceAvailable, + }, + { + name: "stopped instance carries its own reason", + available: &metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionFalse, + Reason: computev1alpha.InstanceAvailableReasonStopped, + Message: "Instance is stopped", + }, + wantStatus: metav1.ConditionFalse, + wantReason: computev1alpha.InstanceAvailableReasonStopped, + wantMessage: "Instance is stopped", + }, + { + name: "unknown availability does not read as healthy", + available: &metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionUnknown, + Reason: computev1alpha.InstanceReadyReasonImageUnavailable, + Message: "Image could not be pulled", + }, + wantStatus: metav1.ConditionFalse, + wantReason: computev1alpha.InstanceReadyReasonImageUnavailable, + wantMessage: "Image could not be pulled", + }, + { + name: "instance that has reported nothing is distinguishable", + available: nil, + wantStatus: metav1.ConditionFalse, + wantReason: holderReasonNotReported, + wantMessage: msgHolderNotReported, + }, + { + name: "unavailable without a message still says something", + available: &metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionFalse, + Reason: computev1alpha.InstanceAvailableReasonStarting, + }, + wantStatus: metav1.ConditionFalse, + wantReason: computev1alpha.InstanceAvailableReasonStarting, + wantMessage: `Instance "holder-test-instance" is not available`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + condition := holderAvailableCondition(newHolderTestInstance(tc.available)) + + assert.Equal(t, networkInterfaceHolderAvailable, condition.Type) + assert.Equal(t, tc.wantStatus, condition.Status) + assert.Equal(t, tc.wantReason, condition.Reason) + assert.Equal(t, tc.wantMessage, condition.Message) + }) + } +} + +// TestReconcileHolderAvailability_BecomesAvailable covers the transition an +// existing interface must pick up on an ordinary reconcile pass. +func TestReconcileHolderAvailability_BecomesAvailable(t *testing.T) { + t.Parallel() + + instance := newHolderTestInstance(&metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionTrue, + Reason: computev1alpha.InstanceAvailableReasonAvailable, + }) + cl := holderTestClient(instance, newHolderTestInterface()) + + r := &InstanceReconciler{NetworkingEnabled: true} + require.NoError(t, r.reconcileHolderAvailability(context.Background(), cl, instance)) + + condition := getHolderCondition(t, cl) + require.NotNil(t, condition) + assert.Equal(t, metav1.ConditionTrue, condition.Status) + assert.Equal(t, computev1alpha.InstanceAvailableReasonAvailable, condition.Reason) + assert.Equal(t, int64(4), condition.ObservedGeneration, + "the interface's generation, not the instance's") +} + +// TestReconcileHolderAvailability_LeavesAvailable covers the drain path: an +// interface already vouched for must be told when the holder stops serving. +func TestReconcileHolderAvailability_LeavesAvailable(t *testing.T) { + t.Parallel() + + instance := newHolderTestInstance(&metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionTrue, + Reason: computev1alpha.InstanceAvailableReasonAvailable, + }) + cl := holderTestClient(instance, newHolderTestInterface()) + + r := &InstanceReconciler{NetworkingEnabled: true} + require.NoError(t, r.reconcileHolderAvailability(context.Background(), cl, instance)) + require.Equal(t, metav1.ConditionTrue, getHolderCondition(t, cl).Status) + + apimeta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionFalse, + Reason: computev1alpha.InstanceAvailableReasonStopping, + Message: "Instance is stopping", + }) + require.NoError(t, r.reconcileHolderAvailability(context.Background(), cl, instance)) + + condition := getHolderCondition(t, cl) + require.NotNil(t, condition) + assert.Equal(t, metav1.ConditionFalse, condition.Status) + assert.Equal(t, computev1alpha.InstanceAvailableReasonStopping, condition.Reason) + assert.Equal(t, "Instance is stopping", condition.Message) +} + +// TestReconcileHolderAvailability_Stable covers convergence: a pass that +// changes nothing must not write, so a steady fleet produces no API traffic and +// no transition timestamp churn. +func TestReconcileHolderAvailability_Stable(t *testing.T) { + t.Parallel() + + instance := newHolderTestInstance(&metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionTrue, + Reason: computev1alpha.InstanceAvailableReasonAvailable, + }) + cl := holderTestClient(instance, newHolderTestInterface()) + + r := &InstanceReconciler{NetworkingEnabled: true} + require.NoError(t, r.reconcileHolderAvailability(context.Background(), cl, instance)) + + var first networkingv1alpha.NetworkInterface + require.NoError(t, cl.Get(context.Background(), client.ObjectKey{ + Namespace: claimTestNamespace, Name: holderTestInterface, + }, &first)) + + require.NoError(t, r.reconcileHolderAvailability(context.Background(), cl, instance)) + + var second networkingv1alpha.NetworkInterface + require.NoError(t, cl.Get(context.Background(), client.ObjectKey{ + Namespace: claimTestNamespace, Name: holderTestInterface, + }, &second)) + + assert.Equal(t, first.ResourceVersion, second.ResourceVersion, + "a no-op pass must not write the interface") + assert.Equal(t, first.Status.Conditions, second.Status.Conditions) +} + +// TestReconcileHolderAvailability_NoInterface covers the two ways there is +// nothing to write to, neither of which is an error. +func TestReconcileHolderAvailability_NoInterface(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + instance func(*computev1alpha.Instance) + objects []client.Object + }{ + { + name: "interface object does not exist", + }, + { + name: "status entry carries no interface reference", + instance: func(i *computev1alpha.Instance) { + i.Status.NetworkInterfaces[0].NetworkInterfaceRef = nil + }, + objects: []client.Object{newHolderTestInterface()}, + }, + { + name: "instance publishes no interfaces at all", + instance: func(i *computev1alpha.Instance) { + i.Status.NetworkInterfaces = nil + }, + objects: []client.Object{newHolderTestInterface()}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + instance := newHolderTestInstance(&metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionTrue, + Reason: computev1alpha.InstanceAvailableReasonAvailable, + }) + if tc.instance != nil { + tc.instance(instance) + } + + cl := holderTestClient(append([]client.Object{instance}, tc.objects...)...) + + r := &InstanceReconciler{NetworkingEnabled: true} + assert.NoError(t, r.reconcileHolderAvailability(context.Background(), cl, instance)) + + if len(tc.objects) > 0 { + assert.Nil(t, getHolderCondition(t, cl)) + } + }) + } +} + +// TestReconcileHolderAvailability_NetworkingDisabled covers a cell without the +// networking CRDs, where the interface kind is not even served. +func TestReconcileHolderAvailability_NetworkingDisabled(t *testing.T) { + t.Parallel() + + instance := newHolderTestInstance(&metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionTrue, + Reason: computev1alpha.InstanceAvailableReasonAvailable, + }) + cl := holderTestClient(instance, newHolderTestInterface()) + + r := &InstanceReconciler{NetworkingEnabled: false} + require.NoError(t, r.reconcileHolderAvailability(context.Background(), cl, instance)) + + assert.Nil(t, getHolderCondition(t, cl)) +} + +// TestHolderAvailableCondition_Terminating covers the case the instance's own +// Available condition cannot express: nothing clears it on the way out, so a +// terminating instance still reports itself available. +func TestHolderAvailableCondition_Terminating(t *testing.T) { + t.Parallel() + + instance := newHolderTestInstance(&metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionTrue, + Reason: computev1alpha.InstanceAvailableReasonAvailable, + }) + deleting := metav1.Now() + instance.DeletionTimestamp = &deleting + + condition := holderAvailableCondition(instance) + + assert.Equal(t, metav1.ConditionFalse, condition.Status) + assert.Equal(t, holderReasonTerminating, condition.Reason) + assert.Equal(t, msgHolderTerminating, condition.Message) +} + +// TestReconcileDeletion_Drains covers the drain path that carries every +// scale-down, rolling update, and redeploy: a terminating instance must stop +// being routed to before its cleanup runs. +func TestReconcileDeletion_Drains(t *testing.T) { + t.Parallel() + + instance := newHolderTestInstance(&metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionTrue, + Reason: computev1alpha.InstanceAvailableReasonAvailable, + }) + + networkInterface := newHolderTestInterface() + apimeta.SetStatusCondition(&networkInterface.Status.Conditions, metav1.Condition{ + Type: networkInterfaceHolderAvailable, + Status: metav1.ConditionTrue, + Reason: computev1alpha.InstanceAvailableReasonAvailable, + }) + + deleting := metav1.Now() + instance.DeletionTimestamp = &deleting + instance.Finalizers = []string{instanceQuotaFinalizer} + + cl := holderTestClient(instance, networkInterface) + + r := &InstanceReconciler{NetworkingEnabled: true} + require.NoError(t, r.reconcileDeletion(context.Background(), cl, "test-cluster", instance)) + + condition := getHolderCondition(t, cl) + require.NotNil(t, condition) + assert.Equal(t, metav1.ConditionFalse, condition.Status) + assert.Equal(t, holderReasonTerminating, condition.Reason) +} + +// TestReconcileDeletion_DrainFailureDoesNotWedge covers the ordering rule: an +// instance that cannot reach its interfaces must still finish deleting, or a +// transient API error strands it in Terminating forever. +func TestReconcileDeletion_DrainFailureDoesNotWedge(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + client func(client.Client) client.Client + objects []client.Object + }{ + { + name: "interface is already gone", + objects: nil, + }, + { + name: "the status write fails", + objects: []client.Object{newHolderTestInterface()}, + client: func(cl client.Client) client.Client { + return &holderFailingStatusClient{Client: cl} + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + instance := newHolderTestInstance(&metav1.Condition{ + Type: computev1alpha.InstanceAvailable, + Status: metav1.ConditionTrue, + Reason: computev1alpha.InstanceAvailableReasonAvailable, + }) + deleting := metav1.Now() + instance.DeletionTimestamp = &deleting + instance.Finalizers = []string{instanceQuotaFinalizer} + + cl := holderTestClient(append([]client.Object{instance}, tc.objects...)...) + if tc.client != nil { + cl = tc.client(cl) + } + + r := &InstanceReconciler{NetworkingEnabled: true} + require.NoError(t, r.reconcileDeletion(context.Background(), cl, "test-cluster", instance), + "a drain failure must not block deletion") + + assert.NotContains(t, instance.Finalizers, instanceQuotaFinalizer, + "the finalizer must still be released") + }) + } +} + +// TestReconcileHolderAvailability_StaleTrueNotInherited covers a reclaimPolicy +// Retain interface outliving the instance that held it: the next instance in +// the slot must not begin life looking healthy on a predecessor's condition. +func TestReconcileHolderAvailability_StaleTrueNotInherited(t *testing.T) { + t.Parallel() + + retained := newHolderTestInterface() + apimeta.SetStatusCondition(&retained.Status.Conditions, metav1.Condition{ + Type: networkInterfaceHolderAvailable, + Status: metav1.ConditionTrue, + Reason: computev1alpha.InstanceAvailableReasonAvailable, + Message: msgInstanceAvailable, + }) + + // The successor has rebound the interface but has not started serving, which + // on a fresh instance means no Available condition at all. + successor := newHolderTestInstance(nil) + successor.Name = "holder-test-instance-successor" + + cl := holderTestClient(successor, retained) + + r := &InstanceReconciler{NetworkingEnabled: true} + require.NoError(t, r.reconcileHolderAvailability(context.Background(), cl, successor)) + + condition := getHolderCondition(t, cl) + require.NotNil(t, condition) + assert.Equal(t, metav1.ConditionFalse, condition.Status) + assert.Equal(t, holderReasonNotReported, condition.Reason) +} + +// holderFailingStatusClient fails every status write, standing in for an +// interface the API server will not let the instance update on its way out. +type holderFailingStatusClient struct { + client.Client +} + +func (c *holderFailingStatusClient) Status() client.SubResourceWriter { + return &holderFailingStatusWriter{SubResourceWriter: c.Client.Status()} +} + +type holderFailingStatusWriter struct { + client.SubResourceWriter +} + +func (w *holderFailingStatusWriter) Update(_ context.Context, _ client.Object, _ ...client.SubResourceUpdateOption) error { + return errors.New("status update refused") +}