diff --git a/pkg/deployd/deployd/deployd.go b/pkg/deployd/deployd/deployd.go index 10e8032c..0edffda4 100644 --- a/pkg/deployd/deployd/deployd.go +++ b/pkg/deployd/deployd/deployd.go @@ -119,8 +119,9 @@ func Run(op *operation.Operation, client kubeclient.Interface) { ) resourceInterface, err := client.ResourceInterface(&resource) + forcedResync := false if err == nil { - _, err = strategy.NewDeployStrategy(resourceInterface).Deploy(op.Context, resource, span) + _, forcedResync, err = strategy.NewDeployStrategy(resourceInterface).Deploy(op.Context, resource, span) } if err != nil { @@ -139,11 +140,11 @@ func Run(op *operation.Operation, client kubeclient.Interface) { op.StatusChan <- pb.NewInProgressStatus(op.Request, "Successfully applied %s", identifier.String()) wait.Add(1) - go func(logger *log.Entry, resource unstructured.Unstructured) { + go func(logger *log.Entry, resource unstructured.Unstructured, forcedResync bool) { deadline, _ := op.Context.Deadline() op.Logger.Debugf("Monitoring rollout status of '%s/%s' in namespace '%s', deadline %s", identifier.GroupVersionKind, identifier.Name, identifier.Namespace, deadline) strat := strategy.NewWatchStrategy(identifier.GroupVersionKind, client) - status := strat.Watch(op, resource, span) + status := strat.Watch(op, resource, span, forcedResync) if status != nil { span.AddEvent(status.Message) if status.GetState().IsError() { @@ -163,7 +164,7 @@ func Run(op *operation.Operation, client kubeclient.Interface) { op.Logger.Debugf("Finished monitoring rollout status of '%s/%s' in namespace '%s'", identifier.GroupVersionKind, identifier.Name, identifier.Namespace) wait.Done() span.End() - }(logger, resource) + }(logger, resource, forcedResync) } op.StatusChan <- pb.NewInProgressStatus(op.Request, "All resources saved to Kubernetes; waiting for completion") diff --git a/pkg/deployd/deployd/deployd_test.go b/pkg/deployd/deployd/deployd_test.go index da10d3f6..78b4f669 100644 --- a/pkg/deployd/deployd/deployd_test.go +++ b/pkg/deployd/deployd/deployd_test.go @@ -40,6 +40,8 @@ type testSpec struct { endStatus *pb.DeploymentStatus // which end state we expect deployedResources []client.Object // list of Kubernetes resources expected to be applied to the cluster - only checks name and namespace processing processCallback // processing that happens in a coroutine together with deployd.Run(). Requires all resources in `deployedResources` to exist. + setup processCallback // processing that happens before deployd.Run() + verify func(t *testing.T, ctx context.Context, rig *testRig, test testSpec) } var tests = []testSpec{ @@ -117,7 +119,7 @@ var tests = []testSpec{ }, }, processing: func(ctx context.Context, rig *testRig, test testSpec) error { - return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.RolloutComplete, "completed", "myapplication")) + return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.RolloutComplete, "completed", "Application", "myapplication")) }, }, @@ -138,7 +140,7 @@ var tests = []testSpec{ }, }, processing: func(ctx context.Context, rig *testRig, test testSpec) error { - return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.FailedSynchronization, "oops", "myapplication-failedsynchronization")) + return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.FailedSynchronization, "oops", "Application", "myapplication-failedsynchronization")) }, }, @@ -159,7 +161,7 @@ var tests = []testSpec{ }, }, processing: func(ctx context.Context, rig *testRig, test testSpec) error { - return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.FailedPrepare, "oops", "myapplication-failedprepare")) + return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.FailedPrepare, "oops", "Application", "myapplication-failedprepare")) }, }, @@ -184,6 +186,177 @@ var tests = []testSpec{ }, deployedResources: nil, }, + + // Redeploying an unchanged Application clears the synchronization hash, + // so that Naiserator synchronizes it instead of skipping it. + { + fixture: "testdata/application-resync.json", + timeout: 5 * time.Second, + endStatus: &pb.DeploymentStatus{ + State: pb.DeploymentState_success, + Message: "Deployment completed successfully.", + }, + deployedResources: []client.Object{ + &nais_io_v1alpha1.Application{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myapplication-resync", + Namespace: "aura", + }, + }, + }, + setup: func(ctx context.Context, rig *testRig, test testSpec) error { + app := &nais_io_v1alpha1.Application{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myapplication-resync", + Namespace: "aura", + }, + Spec: nais_io_v1alpha1.ApplicationSpec{Image: "foo/bar"}, + } + return createWithStatus(ctx, rig, app, &app.Status) + }, + processing: func(ctx context.Context, rig *testRig, test testSpec) error { + return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.RolloutComplete, "completed", "Application", "myapplication-resync")) + }, + verify: func(t *testing.T, ctx context.Context, rig *testRig, test testSpec) { + app := &nais_io_v1alpha1.Application{} + err := rig.client.Get(ctx, client.ObjectKey{Name: "myapplication-resync", Namespace: "aura"}, app) + assert.NoError(t, err) + assertStatusInvalidated(t, app.Status) + }, + }, + + // Naisjobs use the same synchronization hash mechanism as Applications. + { + fixture: "testdata/naisjob-resync.json", + timeout: 5 * time.Second, + endStatus: &pb.DeploymentStatus{ + State: pb.DeploymentState_success, + Message: "Deployment completed successfully.", + }, + deployedResources: []client.Object{ + &nais_io_v1.Naisjob{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mynaisjob-resync", + Namespace: "aura", + }, + }, + }, + setup: func(ctx context.Context, rig *testRig, test testSpec) error { + job := &nais_io_v1.Naisjob{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mynaisjob-resync", + Namespace: "aura", + }, + Spec: nais_io_v1.NaisjobSpec{Image: "foo/bar", Schedule: "*/1 * * * *"}, + } + return createWithStatus(ctx, rig, job, &job.Status) + }, + processing: func(ctx context.Context, rig *testRig, test testSpec) error { + return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.RolloutComplete, "completed", "Naisjob", "mynaisjob-resync")) + }, + verify: func(t *testing.T, ctx context.Context, rig *testRig, test testSpec) { + job := &nais_io_v1.Naisjob{} + err := rig.client.Get(ctx, client.ObjectKey{Name: "mynaisjob-resync", Namespace: "aura"}, job) + assert.NoError(t, err) + assertStatusInvalidated(t, job.Status) + }, + }, + + // A no-op event remains terminal when the update changed generation and did not + // force a resynchronization. + { + fixture: "testdata/application-generation-change.json", + timeout: 5 * time.Second, + endStatus: &pb.DeploymentStatus{ + State: pb.DeploymentState_success, + Message: "Deployment completed successfully.", + }, + deployedResources: []client.Object{ + &nais_io_v1alpha1.Application{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myapplication-generation-change", + Namespace: "aura", + }, + }, + }, + setup: func(ctx context.Context, rig *testRig, test testSpec) error { + app := &nais_io_v1alpha1.Application{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myapplication-generation-change", + Namespace: "aura", + }, + Spec: nais_io_v1alpha1.ApplicationSpec{Image: "foo/old"}, + } + return createWithStatus(ctx, rig, app, &app.Status) + }, + processing: func(ctx context.Context, rig *testRig, test testSpec) error { + return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.RolloutComplete, rolloutMessageNoop, "Application", "myapplication-generation-change")) + }, + verify: func(t *testing.T, ctx context.Context, rig *testRig, test testSpec) { + app := &nais_io_v1alpha1.Application{} + err := rig.client.Get(ctx, client.ObjectKey{Name: "myapplication-generation-change", Namespace: "aura"}, app) + assert.NoError(t, err) + assert.EqualValues(t, 2, app.Generation) + assert.Equal(t, "synchronized-hash", app.Status.SynchronizationHash) + }, + }, + + // Ignore a no-op event emitted before the forced resynchronization completes. + { + fixture: "testdata/application-noop.json", + timeout: 3 * time.Second, + endStatus: &pb.DeploymentStatus{ + State: pb.DeploymentState_failure, + Message: "timeout while waiting for deployment to succeed (total of 1 errors)", + }, + deployedResources: []client.Object{ + &nais_io_v1alpha1.Application{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myapplication-noop", + Namespace: "aura", + }, + }, + }, + setup: func(ctx context.Context, rig *testRig, test testSpec) error { + app := &nais_io_v1alpha1.Application{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myapplication-noop", + Namespace: "aura", + }, + Spec: nais_io_v1alpha1.ApplicationSpec{Image: "foo/bar"}, + } + return createWithStatus(ctx, rig, app, &app.Status) + }, + processing: func(ctx context.Context, rig *testRig, test testSpec) error { + return rig.client.Create(ctx, naiseratorEvent(test.fixture, events.RolloutComplete, rolloutMessageNoop, "Application", "myapplication-noop")) + }, + }, +} + +const rolloutMessageNoop = "No changes; deployment already up to date" + +// createWithStatus persists a workload along with the status Naiserator would have +// written after a successful deployment. Status is a subresource, so it needs a +// separate write. +func createWithStatus(ctx context.Context, rig *testRig, resource client.Object, status *nais_io_v1.Status) error { + err := rig.client.Create(ctx, resource) + if err != nil { + return err + } + + *status = nais_io_v1.Status{ + SynchronizationHash: "synchronized-hash", + SynchronizationState: events.RolloutComplete, + CorrelationID: "previous-deployment", + } + + return rig.client.Status().Update(ctx, resource) +} + +func assertStatusInvalidated(t *testing.T, status nais_io_v1.Status) { + assert.Empty(t, status.SynchronizationHash) + assert.Equal(t, events.RolloutComplete, status.SynchronizationState) + assert.Equal(t, "previous-deployment", status.CorrelationID) } type testRig struct { @@ -460,6 +633,14 @@ func subTest(t *testing.T, rig *testRig, test testSpec, team string) { panic(fmt.Sprintf("test data fixture error in '%s': %s", test.fixture, err)) } + if test.setup != nil { + err = test.setup(ctx, rig, test) + if err != nil { + t.Errorf("Set up fixture: %s", err) + t.FailNow() + } + } + opctx, cancel := context.WithCancel(ctx) defer cancel() @@ -507,9 +688,13 @@ func subTest(t *testing.T, rig *testRig, test testSpec, team string) { assert.NoError(t, err) wg.Wait() + + if test.verify != nil { + test.verify(t, ctx, rig, test) + } } -func naiseratorEvent(id, reason, message, app string) *v1.Event { +func naiseratorEvent(id, reason, message, kind, name string) *v1.Event { return &v1.Event{ ObjectMeta: metav1.ObjectMeta{ Name: "event-" + keygen.RandStringBytes(10), @@ -522,9 +707,9 @@ func naiseratorEvent(id, reason, message, app string) *v1.Event { Reason: reason, Message: message, InvolvedObject: v1.ObjectReference{ - Kind: "Application", + Kind: kind, Namespace: "aura", - Name: app, + Name: name, }, LastTimestamp: metav1.NewTime(time.Now()), } diff --git a/pkg/deployd/deployd/testdata/application-generation-change.json b/pkg/deployd/deployd/testdata/application-generation-change.json new file mode 100644 index 00000000..bef5a3bf --- /dev/null +++ b/pkg/deployd/deployd/testdata/application-generation-change.json @@ -0,0 +1,13 @@ +[ + { + "kind": "Application", + "apiVersion": "nais.io/v1alpha1", + "metadata": { + "name": "myapplication-generation-change", + "namespace": "aura" + }, + "spec": { + "image": "foo/new" + } + } +] diff --git a/pkg/deployd/deployd/testdata/application-noop.json b/pkg/deployd/deployd/testdata/application-noop.json new file mode 100644 index 00000000..a2a57e7e --- /dev/null +++ b/pkg/deployd/deployd/testdata/application-noop.json @@ -0,0 +1,13 @@ +[ + { + "kind": "Application", + "apiVersion": "nais.io/v1alpha1", + "metadata": { + "name": "myapplication-noop", + "namespace": "aura" + }, + "spec": { + "image": "foo/bar" + } + } +] diff --git a/pkg/deployd/deployd/testdata/application-resync.json b/pkg/deployd/deployd/testdata/application-resync.json new file mode 100644 index 00000000..c46c8f49 --- /dev/null +++ b/pkg/deployd/deployd/testdata/application-resync.json @@ -0,0 +1,13 @@ +[ + { + "kind": "Application", + "apiVersion": "nais.io/v1alpha1", + "metadata": { + "name": "myapplication-resync", + "namespace": "aura" + }, + "spec": { + "image": "foo/bar" + } + } +] diff --git a/pkg/deployd/deployd/testdata/naisjob-resync.json b/pkg/deployd/deployd/testdata/naisjob-resync.json new file mode 100644 index 00000000..f91ed259 --- /dev/null +++ b/pkg/deployd/deployd/testdata/naisjob-resync.json @@ -0,0 +1,14 @@ +[ + { + "kind": "Naisjob", + "apiVersion": "nais.io/v1", + "metadata": { + "name": "mynaisjob-resync", + "namespace": "aura" + }, + "spec": { + "image": "foo/bar", + "schedule": "*/1 * * * *" + } + } +] diff --git a/pkg/deployd/strategy/deploy.go b/pkg/deployd/strategy/deploy.go index 9d93bab0..7ee6898c 100644 --- a/pkg/deployd/strategy/deploy.go +++ b/pkg/deployd/strategy/deploy.go @@ -9,6 +9,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" ) @@ -17,25 +18,25 @@ func NewDeployStrategy(namespacedResource dynamic.ResourceInterface) DeployStrat } type DeployStrategy interface { - Deploy(ctx context.Context, resource unstructured.Unstructured, trace trace.Span) (*unstructured.Unstructured, error) + Deploy(ctx context.Context, resource unstructured.Unstructured, trace trace.Span) (*unstructured.Unstructured, bool, error) } type createOrUpdateStrategy struct { client dynamic.ResourceInterface } -func (c createOrUpdateStrategy) Deploy(ctx context.Context, resource unstructured.Unstructured, trace trace.Span) (*unstructured.Unstructured, error) { +func (c createOrUpdateStrategy) Deploy(ctx context.Context, resource unstructured.Unstructured, trace trace.Span) (*unstructured.Unstructured, bool, error) { existing, err := c.client.Get(ctx, resource.GetName(), metav1.GetOptions{}) if errors.IsNotFound(err) { deployed, err := c.client.Create(ctx, &resource, metav1.CreateOptions{ FieldValidation: metav1.FieldValidationStrict, }) if err != nil { - return nil, fmt.Errorf("creating resource: %w", transformStrictDecodingError(resource, err)) + return nil, false, fmt.Errorf("creating resource: %w", transformStrictDecodingError(resource, err)) } - return deployed, nil + return deployed, false, nil } else if err != nil { - return nil, fmt.Errorf("get existing resource: %w", err) + return nil, false, fmt.Errorf("get existing resource: %w", err) } resource.SetResourceVersion(existing.GetResourceVersion()) @@ -43,10 +44,31 @@ func (c createOrUpdateStrategy) Deploy(ctx context.Context, resource unstructure FieldValidation: metav1.FieldValidationStrict, }) if err != nil { - return nil, fmt.Errorf("updating resource: %w", transformStrictDecodingError(resource, err)) + return nil, false, fmt.Errorf("updating resource: %w", transformStrictDecodingError(resource, err)) } - return updated, nil + forcedResync := shouldInvalidateSynchronizationHash(existing, updated) + if forcedResync { + // Patch after Update so Naiserator sees this deployment's spec and correlation + // ID annotation without replacing other status fields. + err = c.invalidateSynchronizationHash(ctx, resource.GetName()) + if err != nil { + return nil, false, fmt.Errorf("invalidating synchronization hash: %w", err) + } + trace.AddEvent("Forced resynchronization of Nais resource") + } + + return updated, forcedResync, nil +} + +func shouldInvalidateSynchronizationHash(existing, updated *unstructured.Unstructured) bool { + return isNaisWorkload(updated.GroupVersionKind()) && updated.GetGeneration() == existing.GetGeneration() +} + +func (c createOrUpdateStrategy) invalidateSynchronizationHash(ctx context.Context, name string) error { + _, err := c.client.Patch(ctx, name, types.MergePatchType, + []byte(`{"status":{"synchronizationHash":null}}`), metav1.PatchOptions{}, "status") + return err } func transformStrictDecodingError(resource unstructured.Unstructured, err error) error { diff --git a/pkg/deployd/strategy/deploy_test.go b/pkg/deployd/strategy/deploy_test.go new file mode 100644 index 00000000..4dec2a80 --- /dev/null +++ b/pkg/deployd/strategy/deploy_test.go @@ -0,0 +1,178 @@ +package strategy + +import ( + "testing" + + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/dynamic/fake" + k8stesting "k8s.io/client-go/testing" +) + +var ( + applicationGVK = schema.GroupVersionKind{Group: "nais.io", Version: "v1alpha1", Kind: "Application"} + applicationGVR = schema.GroupVersionResource{Group: "nais.io", Version: "v1alpha1", Resource: "applications"} + naisjobGVK = schema.GroupVersionKind{Group: "nais.io", Version: "v1", Kind: "Naisjob"} + naisjobGVR = schema.GroupVersionResource{Group: "nais.io", Version: "v1", Resource: "naisjobs"} +) + +func TestDeployInvalidatesSynchronizationHash(t *testing.T) { + tests := []struct { + name string + gvk schema.GroupVersionKind + gvr schema.GroupVersionResource + }{ + {name: "Application", gvk: applicationGVK, gvr: applicationGVR}, + {name: "Naisjob", gvk: naisjobGVK, gvr: naisjobGVR}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + existing := testResource(tt.gvk) + existing.SetGeneration(1) + require.NoError(t, unstructured.SetNestedField(existing.Object, "current-hash", "status", "synchronizationHash")) + desired := existing.DeepCopy() + + client := fake.NewSimpleDynamicClient(runtime.NewScheme(), &existing) + resourceClient := client.Resource(tt.gvr).Namespace(existing.GetNamespace()) + + deployed, forcedResync, err := NewDeployStrategy(resourceClient).Deploy(t.Context(), *desired, noop.Span{}) + require.NoError(t, err) + require.NotNil(t, deployed) + require.True(t, forcedResync) + + // The spec must be written before the hash is cleared, so that the + // resynchronization picks up this deployment rather than the previous one. + require.Equal(t, []string{"get", "update", "patch"}, verbs(client.Actions())) + + patch := lastPatch(t, client.Actions()) + require.Equal(t, "status", patch.GetSubresource()) + require.Equal(t, types.MergePatchType, patch.GetPatchType()) + require.JSONEq(t, `{"status":{"synchronizationHash":null}}`, string(patch.GetPatch())) + }) + } +} + +func TestDeployDoesNotInvalidateSynchronizationHash(t *testing.T) { + tests := []struct { + name string + gvk schema.GroupVersionKind + gvr schema.GroupVersionResource + existing bool + }{ + { + name: "new Application is synchronized on creation", + gvk: applicationGVK, + gvr: applicationGVR, + }, + { + name: "ConfigMap has no synchronization hash", + gvk: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"}, + gvr: schema.GroupVersionResource{Version: "v1", Resource: "configmaps"}, + existing: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resource := testResource(tt.gvk) + client := fake.NewSimpleDynamicClient(runtime.NewScheme()) + resourceClient := client.Resource(tt.gvr).Namespace(resource.GetNamespace()) + if tt.existing { + _, err := resourceClient.Create(t.Context(), &resource, metav1.CreateOptions{}) + require.NoError(t, err) + client.ClearActions() + } + + _, forcedResync, err := NewDeployStrategy(resourceClient).Deploy(t.Context(), resource, noop.Span{}) + require.NoError(t, err) + require.False(t, forcedResync) + require.NotContains(t, verbs(client.Actions()), "patch") + }) + } +} + +func TestDeployDoesNotInvalidateSynchronizationHashWhenGenerationChanges(t *testing.T) { + tests := []struct { + name string + gvk schema.GroupVersionKind + gvr schema.GroupVersionResource + }{ + {name: "Application", gvk: applicationGVK, gvr: applicationGVR}, + {name: "Naisjob", gvk: naisjobGVK, gvr: naisjobGVR}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + existing := testResource(tt.gvk) + existing.SetGeneration(1) + desired := existing.DeepCopy() + + client := fake.NewSimpleDynamicClient(runtime.NewScheme(), &existing) + client.PrependReactor("update", tt.gvr.Resource, func(action k8stesting.Action) (bool, runtime.Object, error) { + updated := action.(k8stesting.UpdateAction).GetObject().(*unstructured.Unstructured).DeepCopy() + updated.SetGeneration(2) + return true, updated, nil + }) + resourceClient := client.Resource(tt.gvr).Namespace(existing.GetNamespace()) + + deployed, forcedResync, err := NewDeployStrategy(resourceClient).Deploy(t.Context(), *desired, noop.Span{}) + require.NoError(t, err) + require.EqualValues(t, 2, deployed.GetGeneration()) + require.False(t, forcedResync) + require.Equal(t, []string{"get", "update"}, verbs(client.Actions())) + }) + } +} + +func TestDeployFailsWhenSynchronizationHashCannotBeInvalidated(t *testing.T) { + existing := testResource(applicationGVK) + desired := existing.DeepCopy() + + client := fake.NewSimpleDynamicClient(runtime.NewScheme(), &existing) + client.PrependReactor("patch", "applications", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.NewForbidden(applicationGVR.GroupResource(), existing.GetName(), nil) + }) + resourceClient := client.Resource(applicationGVR).Namespace(existing.GetNamespace()) + + _, forcedResync, err := NewDeployStrategy(resourceClient).Deploy(t.Context(), *desired, noop.Span{}) + require.ErrorContains(t, err, "invalidating synchronization hash") + require.True(t, errors.IsForbidden(err)) + require.False(t, forcedResync) +} + +func verbs(actions []k8stesting.Action) []string { + found := make([]string, 0, len(actions)) + for _, action := range actions { + found = append(found, action.GetVerb()) + } + return found +} + +func lastPatch(t *testing.T, actions []k8stesting.Action) k8stesting.PatchAction { + t.Helper() + for i := len(actions) - 1; i >= 0; i-- { + if patch, ok := actions[i].(k8stesting.PatchAction); ok { + return patch + } + } + t.Fatal("expected a patch action") + return nil +} + +func testResource(gvk schema.GroupVersionKind) unstructured.Unstructured { + resource := unstructured.Unstructured{Object: map[string]any{ + "metadata": map[string]any{ + "name": "test-resource", + "namespace": "test-namespace", + }, + }} + resource.SetGroupVersionKind(gvk) + return resource +} diff --git a/pkg/deployd/strategy/deploymentwatch.go b/pkg/deployd/strategy/deploymentwatch.go index 8af98fb4..487fae33 100644 --- a/pkg/deployd/strategy/deploymentwatch.go +++ b/pkg/deployd/strategy/deploymentwatch.go @@ -21,7 +21,7 @@ type deployment struct { client kubeclient.Interface } -func (d deployment) Watch(op *operation.Operation, resource unstructured.Unstructured, trace trace.Span) *pb.DeploymentStatus { +func (d deployment) Watch(op *operation.Operation, resource unstructured.Unstructured, trace trace.Span, _ bool) *pb.DeploymentStatus { var cur *apps.Deployment var nova *apps.Deployment var err error diff --git a/pkg/deployd/strategy/jobwatch.go b/pkg/deployd/strategy/jobwatch.go index 0e6b51e6..eeb32380 100644 --- a/pkg/deployd/strategy/jobwatch.go +++ b/pkg/deployd/strategy/jobwatch.go @@ -18,7 +18,7 @@ type job struct { client kubeclient.Interface } -func (j job) Watch(op *operation.Operation, resource unstructured.Unstructured, trace trace.Span) *pb.DeploymentStatus { +func (j job) Watch(op *operation.Operation, resource unstructured.Unstructured, trace trace.Span, _ bool) *pb.DeploymentStatus { var job *v1.Job var err error diff --git a/pkg/deployd/strategy/nais.go b/pkg/deployd/strategy/nais.go index bd487480..b5bb73ae 100644 --- a/pkg/deployd/strategy/nais.go +++ b/pkg/deployd/strategy/nais.go @@ -23,7 +23,12 @@ type naisResource struct { client kubeclient.Interface } -func (a naisResource) Watch(op *operation.Operation, resource unstructured.Unstructured, trace trace.Span) *pb.DeploymentStatus { +// Naiserator may emit this before deployd clears the hash for an unchanged generation. +// Ignore it because it does not represent the forced resynchronization. +// https://github.com/nais/naiserator/blob/master/pkg/synchronizer/monitoring.go +const rolloutMessageNoop = "No changes; deployment already up to date" + +func (a naisResource) Watch(op *operation.Operation, resource unstructured.Unstructured, trace trace.Span, forcedResync bool) *pb.DeploymentStatus { var err error eventsClient := a.client.Kubernetes().CoreV1().Events(resource.GetNamespace()) @@ -73,6 +78,11 @@ func (a naisResource) Watch(op *operation.Operation, resource unstructured.Unstr continue } + if forcedResync && event.ReportingController == "naiserator" && event.Reason == events.RolloutComplete && event.Message == rolloutMessageNoop { + op.Logger.Tracef("Ignoring no-op rollout event %s; awaiting forced resynchronization", event.Name) + continue + } + status := StatusFromEvent(event, op.Request) if status == nil { return pb.NewFailureStatus(op.Request, fmt.Errorf("this application has been redeployed, aborting monitoring")) diff --git a/pkg/deployd/strategy/watch.go b/pkg/deployd/strategy/watch.go index bbc5db76..6b855666 100644 --- a/pkg/deployd/strategy/watch.go +++ b/pkg/deployd/strategy/watch.go @@ -18,18 +18,18 @@ var ( ) type WatchStrategy interface { - Watch(op *operation.Operation, resource unstructured.Unstructured, trace trace.Span) *pb.DeploymentStatus + Watch(op *operation.Operation, resource unstructured.Unstructured, trace trace.Span, forcedResync bool) *pb.DeploymentStatus } type NoOp struct{} -func (c NoOp) Watch(op *operation.Operation, resource unstructured.Unstructured, trace trace.Span) *pb.DeploymentStatus { +func (c NoOp) Watch(op *operation.Operation, resource unstructured.Unstructured, trace trace.Span, _ bool) *pb.DeploymentStatus { op.Logger.Debugf("Watch not implemented for resource %s/%s", resource.GroupVersionKind().String(), resource.GetName()) return nil } func NewWatchStrategy(gvk schema.GroupVersionKind, client kubeclient.Interface) WatchStrategy { - if gvk.Group == "nais.io" && (gvk.Kind == "Application" || gvk.Kind == "Naisjob") { + if isNaisWorkload(gvk) { return naisResource{client: client} } @@ -43,3 +43,7 @@ func NewWatchStrategy(gvk schema.GroupVersionKind, client kubeclient.Interface) return NoOp{} } + +func isNaisWorkload(gvk schema.GroupVersionKind) bool { + return gvk.Group == "nais.io" && (gvk.Kind == "Application" || gvk.Kind == "Naisjob") +}