Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion internal/controller/nodedisruption_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,14 @@ func (r *NodeDisruptionReconciler) Reconcile(ctx context.Context, req ctrl.Reque

if !reflect.DeepEqual(nd.Status, reconciler.NodeDisruption.Status) {
logger.Info("Updating Status, done with", "state", reconciler.NodeDisruption.Status.State)
return clusterResult, reconciler.UpdateStatus(ctx)
if err := reconciler.UpdateStatus(ctx); err != nil {
Comment thread
damsallem marked this conversation as resolved.
if errors.IsNotFound(err) {
logger.Info("NodeDisruption was deleted before status update, skipping", "error", err)
return clusterResult, nil
}
return clusterResult, err
}
return clusterResult, nil
}
logger.Info("Reconciliation successful", "state", reconciler.NodeDisruption.Status.State)
return clusterResult, nil
Expand Down
60 changes: 60 additions & 0 deletions internal/controller/nodedisruption_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,26 @@ import (
"io"
"net/http"
"net/http/httptest"
"testing"
"time"

nodedisruptionv1alpha1 "github.com/criteo/node-disruption-controller/api/v1alpha1"
"github.com/criteo/node-disruption-controller/internal/appmgr"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
errorsk8s "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"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"
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
)

// +kubebuilder:docs-gen:collapse=Imports
Expand Down Expand Up @@ -1159,3 +1165,57 @@ var _ = Describe("NodeDisruption controller", func() {
})
})
})

// TestReconcile_DoesNotErrorOnNotFoundStatusUpdate verifies that when the
// NodeDisruption is removed between Reconcile's Get and the status update
// (e.g., the object's finalizer was removed and it was deleted from etcd),
// Reconcile swallows the resulting NotFound error instead of bubbling it up to
// the controller-runtime workqueue (which would log it as a Reconciler error
// and trigger an unnecessary requeue).
func TestReconcile_DoesNotErrorOnNotFoundStatusUpdate(t *testing.T) {
s := runtime.NewScheme()
require.NoError(t, scheme.AddToScheme(s))
require.NoError(t, nodedisruptionv1alpha1.AddToScheme(s))

nd := &nodedisruptionv1alpha1.NodeDisruption{
ObjectMeta: metav1.ObjectMeta{
Name: "vanishing-nd",
Namespace: "default",
Finalizers: []string{FinalizerName},
},
Spec: nodedisruptionv1alpha1.NodeDisruptionSpec{
NodeSelector: metav1.LabelSelector{},
Type: "maintenance",
},
}

notFound := errorsk8s.NewNotFound(
nodedisruptionv1alpha1.GroupVersion.WithResource("nodedisruptions").GroupResource(),
nd.Name,
)

c := fake.NewClientBuilder().
WithScheme(s).
WithObjects(nd).
WithStatusSubresource(&nodedisruptionv1alpha1.NodeDisruption{}).
WithInterceptorFuncs(interceptor.Funcs{
SubResourceUpdate: func(_ context.Context, _ client.Client, subResourceName string, _ client.Object, _ ...client.SubResourceUpdateOption) error {
if subResourceName == "status" {
return notFound
}
return nil
},
}).
Build()

r := &NodeDisruptionReconciler{
Client: c,
Scheme: s,
Config: NodeDisruptionReconcilerConfig{RetryInterval: time.Second},
}

_, err := r.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{Name: nd.Name, Namespace: nd.Namespace},
})
assert.NoError(t, err, "NotFound on status update must be swallowed by Reconcile")
}
Loading