From f5253eb33423a6260f4157acb3dca69d0efe7c96 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Tue, 12 Jun 2018 14:36:42 +0100 Subject: [PATCH 1/3] Add multidoc test data and reconcile tests This included making some tests that passed by coincidence pass by design. --- cluster/kubernetes/resource/load_test.go | 4 +- cluster/kubernetes/testfiles/data.go | 54 +++++++++++- daemon/daemon_test.go | 4 +- daemon/loop_test.go | 59 ++++++------- release/releaser_test.go | 101 +++++++++++------------ 5 files changed, 135 insertions(+), 87 deletions(-) diff --git a/cluster/kubernetes/resource/load_test.go b/cluster/kubernetes/resource/load_test.go index a4dec2611..8720539f1 100644 --- a/cluster/kubernetes/resource/load_test.go +++ b/cluster/kubernetes/resource/load_test.go @@ -204,8 +204,8 @@ func TestLoadSome(t *testing.T) { if err != nil { t.Error(err) } - if len(objs) != len(testfiles.ServiceMap(dir)) { - t.Errorf("expected %d objects from %d files, got result:\n%#v", len(testfiles.ServiceMap(dir)), len(testfiles.Files), objs) + if len(objs) != len(testfiles.ResourceMap) { + t.Errorf("expected %d objects from %d files, got result:\n%#v", len(testfiles.ResourceMap), len(testfiles.Files), objs) } } diff --git a/cluster/kubernetes/testfiles/data.go b/cluster/kubernetes/testfiles/data.go index aec8c12ec..322be2317 100644 --- a/cluster/kubernetes/testfiles/data.go +++ b/cluster/kubernetes/testfiles/data.go @@ -42,13 +42,25 @@ func WriteTestFiles(dir string) error { // ----- DATA -// ServiceMap ... given a base path, construct the map representing the services -// given in the test data. +// ResourceMap is the map of resource names to relative paths, which +// must correspond with `Files` below. +var ResourceMap = map[flux.ResourceID]string{ + flux.MustParseResourceID("default:deployment/helloworld"): "helloworld-deploy.yaml", + flux.MustParseResourceID("default:deployment/locked-service"): "locked-service-deploy.yaml", + flux.MustParseResourceID("default:deployment/test-service"): "test/test-service-deploy.yaml", + flux.MustParseResourceID("default:deployment/www-example-io"): "multi.yaml", + flux.MustParseResourceID("default:service/www-example-service"): "multi.yaml", +} + +// ServiceMap ... given a base path, construct the map representing +// the services given in the test data. Must be kept in sync with +// `Files` below. TODO(michael): derive from ResourceMap, or similar. func ServiceMap(dir string) map[flux.ResourceID][]string { return map[flux.ResourceID][]string{ flux.MustParseResourceID("default:deployment/helloworld"): []string{filepath.Join(dir, "helloworld-deploy.yaml")}, flux.MustParseResourceID("default:deployment/locked-service"): []string{filepath.Join(dir, "locked-service-deploy.yaml")}, flux.MustParseResourceID("default:deployment/test-service"): []string{filepath.Join(dir, "test/test-service-deploy.yaml")}, + flux.MustParseResourceID("default:deployment/www-example-io"): []string{filepath.Join(dir, "multi.yaml")}, } } @@ -123,6 +135,44 @@ spec: ports: - containerPort: 80 `, + // A multidoc, since we support those now + "multi.yaml": `apiVersion: apps/v1beta1 +kind: Deployment +metadata: + annotations: + flux.weave.works/automated: "true" + name: www-example-io +spec: + replicas: 1 + template: + metadata: + labels: + app : www-example-io + spec: + containers: + - name: www-example-io + image: quay.io/weaveworks/helloworld:master-a000001 + imagePullPolicy: Always + ports: + - containerPort: 80 + imagePullSecrets: + - name: imagesecret +--- +apiVersion: v1 +kind: Service +metadata: + labels: + app: www-example-io + name: www-example-service +spec: + type: NodePort + ports: + - name: www-example-io + port: 80 + protocol: TCP + selector: + app: www-example-io +`, // A tricksy chart directory, which should be skipped entirely. Adapted from // https://github.com/kubernetes/helm/tree/master/docs/examples diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index f8f9baa2e..71168f97e 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -329,8 +329,8 @@ func TestDaemon_NotifyChange(t *testing.T) { t.Errorf("Sync was not called once, was called %d times", syncCalled) } else if syncDef == nil { t.Errorf("Sync was called with a nil syncDef") - } else if len(syncDef.Actions) != len(testfiles.ServiceMap("unimportant")) { - t.Errorf("Sync was not called with the %d actions, was called with: %d", len(testfiles.Files), len(syncDef.Actions)) + } else if len(syncDef.Actions) != len(testfiles.ResourceMap) { + t.Errorf("Expected Sync called with %d actions (resources), was called with %d", len(testfiles.ResourceMap), len(syncDef.Actions)) } // Check that history was written to diff --git a/daemon/loop_test.go b/daemon/loop_test.go index f9cf1c51c..feb0ef6bc 100644 --- a/daemon/loop_test.go +++ b/daemon/loop_test.go @@ -17,6 +17,7 @@ import ( "github.com/weaveworks/flux/cluster" "github.com/weaveworks/flux/cluster/kubernetes" kresource "github.com/weaveworks/flux/cluster/kubernetes/resource" + "github.com/weaveworks/flux/cluster/kubernetes/testfiles" "github.com/weaveworks/flux/event" "github.com/weaveworks/flux/git" "github.com/weaveworks/flux/git/gittest" @@ -97,11 +98,11 @@ func TestPullAndSync_InitialSync(t *testing.T) { syncCalled := 0 var syncDef *cluster.SyncDef - expectedServiceIDs := flux.ResourceIDs{ - flux.MustParseResourceID("default:deployment/locked-service"), - flux.MustParseResourceID("default:deployment/test-service"), - flux.MustParseResourceID("default:deployment/helloworld")} - expectedServiceIDs.Sort() + expectedResourceIDs := flux.ResourceIDs{} + for id, _ := range testfiles.ResourceMap { + expectedResourceIDs = append(expectedResourceIDs, id) + } + expectedResourceIDs.Sort() k8s.SyncFunc = func(def cluster.SyncDef) error { syncCalled++ syncDef = &def @@ -115,8 +116,8 @@ func TestPullAndSync_InitialSync(t *testing.T) { t.Errorf("Sync was not called once, was called %d times", syncCalled) } else if syncDef == nil { t.Errorf("Sync was called with a nil syncDef") - } else if len(syncDef.Actions) != len(expectedServiceIDs) { - t.Errorf("Sync was not called with the %d actions, was called with: %d", len(expectedServiceIDs)*2, len(syncDef.Actions)) + } else if len(syncDef.Actions) != len(expectedResourceIDs) { + t.Errorf("Sync was not called with %d actions (resources), was called with %d", len(expectedResourceIDs), len(syncDef.Actions)) } // The emitted event has all service ids @@ -128,10 +129,10 @@ func TestPullAndSync_InitialSync(t *testing.T) { } else if es[0].Type != event.EventSync { t.Errorf("Unexpected event type: %#v", es[0]) } else { - gotServiceIDs := es[0].ServiceIDs - flux.ResourceIDs(gotServiceIDs).Sort() - if !reflect.DeepEqual(gotServiceIDs, []flux.ResourceID(expectedServiceIDs)) { - t.Errorf("Unexpected event service ids: %#v, expected: %#v", gotServiceIDs, expectedServiceIDs) + gotResourceIDs := es[0].ServiceIDs + flux.ResourceIDs(gotResourceIDs).Sort() + if !reflect.DeepEqual(gotResourceIDs, []flux.ResourceID(expectedResourceIDs)) { + t.Errorf("Unexpected event service ids: %#v, expected: %#v", gotResourceIDs, expectedResourceIDs) } } // It creates the tag at HEAD @@ -166,11 +167,11 @@ func TestDoSync_NoNewCommits(t *testing.T) { syncCalled := 0 var syncDef *cluster.SyncDef - expectedServiceIDs := flux.ResourceIDs{ - flux.MustParseResourceID("default:deployment/locked-service"), - flux.MustParseResourceID("default:deployment/test-service"), - flux.MustParseResourceID("default:deployment/helloworld")} - expectedServiceIDs.Sort() + expectedResourceIDs := flux.ResourceIDs{} + for id, _ := range testfiles.ResourceMap { + expectedResourceIDs = append(expectedResourceIDs, id) + } + expectedResourceIDs.Sort() k8s.SyncFunc = func(def cluster.SyncDef) error { syncCalled++ syncDef = &def @@ -186,8 +187,8 @@ func TestDoSync_NoNewCommits(t *testing.T) { t.Errorf("Sync was not called once, was called %d times", syncCalled) } else if syncDef == nil { t.Errorf("Sync was called with a nil syncDef") - } else if len(syncDef.Actions) != len(expectedServiceIDs) { - t.Errorf("Sync was not called with the %d actions, was called with: %d", len(expectedServiceIDs)*2, len(syncDef.Actions)) + } else if len(syncDef.Actions) != len(expectedResourceIDs) { + t.Errorf("Sync was not called with %d actions, was called with: %d", len(expectedResourceIDs), len(syncDef.Actions)) } // The emitted event has no service ids @@ -259,11 +260,11 @@ func TestDoSync_WithNewCommit(t *testing.T) { syncCalled := 0 var syncDef *cluster.SyncDef - expectedServiceIDs := flux.ResourceIDs{ - flux.MustParseResourceID("default:deployment/locked-service"), - flux.MustParseResourceID("default:deployment/test-service"), - flux.MustParseResourceID("default:deployment/helloworld")} - expectedServiceIDs.Sort() + expectedResourceIDs := flux.ResourceIDs{} + for id, _ := range testfiles.ResourceMap { + expectedResourceIDs = append(expectedResourceIDs, id) + } + expectedResourceIDs.Sort() k8s.SyncFunc = func(def cluster.SyncDef) error { syncCalled++ syncDef = &def @@ -277,8 +278,8 @@ func TestDoSync_WithNewCommit(t *testing.T) { t.Errorf("Sync was not called once, was called %d times", syncCalled) } else if syncDef == nil { t.Errorf("Sync was called with a nil syncDef") - } else if len(syncDef.Actions) != len(expectedServiceIDs) { - t.Errorf("Sync was not called with the %d actions, was called with: %d", len(expectedServiceIDs)*2, len(syncDef.Actions)) + } else if len(syncDef.Actions) != len(expectedResourceIDs) { + t.Errorf("Sync was not called with %d actions, was called with %d", len(expectedResourceIDs), len(syncDef.Actions)) } // The emitted event has no service ids @@ -290,11 +291,11 @@ func TestDoSync_WithNewCommit(t *testing.T) { } else if es[0].Type != event.EventSync { t.Errorf("Unexpected event type: %#v", es[0]) } else { - gotServiceIDs := es[0].ServiceIDs - flux.ResourceIDs(gotServiceIDs).Sort() + gotResourceIDs := es[0].ServiceIDs + flux.ResourceIDs(gotResourceIDs).Sort() // Event should only have changed service ids - if !reflect.DeepEqual(gotServiceIDs, []flux.ResourceID{flux.MustParseResourceID("default:deployment/helloworld")}) { - t.Errorf("Unexpected event service ids: %#v, expected: %#v", gotServiceIDs, []flux.ResourceID{flux.MustParseResourceID("default/helloworld")}) + if !reflect.DeepEqual(gotResourceIDs, []flux.ResourceID{flux.MustParseResourceID("default:deployment/helloworld")}) { + t.Errorf("Unexpected event service ids: %#v, expected: %#v", gotResourceIDs, []flux.ResourceID{flux.MustParseResourceID("default:deployment/helloworld")}) } } // It moves the tag diff --git a/release/releaser_test.go b/release/releaser_test.go index 932764f96..19c3a1d0d 100644 --- a/release/releaser_test.go +++ b/release/releaser_test.go @@ -135,6 +135,31 @@ func setup(t *testing.T) (*git.Checkout, func()) { return gittest.Checkout(t) } +var ignoredNotIncluded = update.ControllerResult{ + Status: update.ReleaseStatusIgnored, + Error: update.NotIncluded, +} + +var ignoredNotInRepo = update.ControllerResult{ + Status: update.ReleaseStatusIgnored, + Error: update.NotInRepo, +} + +var ignoredNotInCluster = update.ControllerResult{ + Status: update.ReleaseStatusIgnored, + Error: update.NotInCluster, +} + +var skippedNotInCluster = update.ControllerResult{ + Status: update.ReleaseStatusSkipped, + Error: update.NotInCluster, +} + +var skippedNotInRepo = update.ControllerResult{ + Status: update.ReleaseStatusSkipped, + Error: update.NotInRepo, +} + func Test_FilterLogic(t *testing.T) { cluster := mockCluster(hwSvc, lockedSvc) // no testsvc in cluster, but it _is_ in repo notInRepoService := "default:deployment/notInRepo" @@ -169,14 +194,9 @@ func Test_FilterLogic(t *testing.T) { }, }, }, - flux.MustParseResourceID("default:deployment/locked-service"): update.ControllerResult{ - Status: update.ReleaseStatusIgnored, - Error: update.NotIncluded, - }, - flux.MustParseResourceID("default:deployment/test-service"): update.ControllerResult{ - Status: update.ReleaseStatusIgnored, - Error: update.NotIncluded, - }, + flux.MustParseResourceID("default:deployment/locked-service"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/test-service"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/www-example-io"): ignoredNotIncluded, }, }, { Name: "exclude specific service", @@ -206,10 +226,8 @@ func Test_FilterLogic(t *testing.T) { Status: update.ReleaseStatusIgnored, Error: update.Excluded, }, - flux.MustParseResourceID("default:deployment/test-service"): update.ControllerResult{ - Status: update.ReleaseStatusSkipped, - Error: update.NotInCluster, - }, + flux.MustParseResourceID("default:deployment/test-service"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/www-example-io"): skippedNotInCluster, }, }, { Name: "update specific image", @@ -238,6 +256,10 @@ func Test_FilterLogic(t *testing.T) { Status: update.ReleaseStatusSkipped, Error: update.NotInCluster, }, + flux.MustParseResourceID("default:deployment/www-example-io"): update.ControllerResult{ + Status: update.ReleaseStatusSkipped, + Error: update.NotInCluster, + }, }, }, // skipped if: not ignored AND (locked or not found in cluster) @@ -270,10 +292,8 @@ func Test_FilterLogic(t *testing.T) { Status: update.ReleaseStatusSkipped, Error: update.Locked, }, - flux.MustParseResourceID("default:deployment/test-service"): update.ControllerResult{ - Status: update.ReleaseStatusSkipped, - Error: update.NotInCluster, - }, + flux.MustParseResourceID("default:deployment/test-service"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/www-example-io"): skippedNotInCluster, }, }, { @@ -304,10 +324,8 @@ func Test_FilterLogic(t *testing.T) { Status: update.ReleaseStatusSkipped, Error: update.Locked, }, - flux.MustParseResourceID("default:deployment/test-service"): update.ControllerResult{ - Status: update.ReleaseStatusSkipped, - Error: update.NotInCluster, - }, + flux.MustParseResourceID("default:deployment/test-service"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/www-example-io"): skippedNotInCluster, }, }, { @@ -319,22 +337,11 @@ func Test_FilterLogic(t *testing.T) { Excludes: []flux.ResourceID{}, }, Expected: update.Result{ - flux.MustParseResourceID("default:deployment/helloworld"): update.ControllerResult{ - Status: update.ReleaseStatusIgnored, - Error: update.NotIncluded, - }, - flux.MustParseResourceID("default:deployment/locked-service"): update.ControllerResult{ - Status: update.ReleaseStatusIgnored, - Error: update.NotIncluded, - }, - flux.MustParseResourceID("default:deployment/test-service"): update.ControllerResult{ - Status: update.ReleaseStatusIgnored, - Error: update.NotIncluded, - }, - flux.MustParseResourceID(notInRepoService): update.ControllerResult{ - Status: update.ReleaseStatusSkipped, - Error: update.NotInRepo, - }, + flux.MustParseResourceID("default:deployment/helloworld"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/locked-service"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/test-service"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/www-example-io"): ignoredNotIncluded, + flux.MustParseResourceID(notInRepoService): skippedNotInRepo, }, }, } { @@ -379,14 +386,9 @@ func Test_ImageStatus(t *testing.T) { Excludes: []flux.ResourceID{}, }, Expected: update.Result{ - flux.MustParseResourceID("default:deployment/helloworld"): update.ControllerResult{ - Status: update.ReleaseStatusIgnored, - Error: update.NotIncluded, - }, - flux.MustParseResourceID("default:deployment/locked-service"): update.ControllerResult{ - Status: update.ReleaseStatusIgnored, - Error: update.NotIncluded, - }, + flux.MustParseResourceID("default:deployment/helloworld"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/locked-service"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/www-example-io"): ignoredNotIncluded, flux.MustParseResourceID("default:deployment/test-service"): update.ControllerResult{ Status: update.ReleaseStatusIgnored, Error: update.DoesNotUseImage, @@ -405,14 +407,9 @@ func Test_ImageStatus(t *testing.T) { Status: update.ReleaseStatusSkipped, Error: update.ImageUpToDate, }, - flux.MustParseResourceID("default:deployment/locked-service"): update.ControllerResult{ - Status: update.ReleaseStatusIgnored, - Error: update.NotIncluded, - }, - flux.MustParseResourceID("default:deployment/test-service"): update.ControllerResult{ - Status: update.ReleaseStatusIgnored, - Error: update.NotIncluded, - }, + flux.MustParseResourceID("default:deployment/locked-service"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/test-service"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/www-example-io"): ignoredNotIncluded, }, }, } { From e9a95a334adae4ed5f5f5f1e296f9d9d11d07dea Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Tue, 12 Jun 2018 14:43:37 +0100 Subject: [PATCH 2/3] Use testing.T.Run(...) to run subtests --- release/releaser_test.go | 42 ++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/release/releaser_test.go b/release/releaser_test.go index 19c3a1d0d..4664ac596 100644 --- a/release/releaser_test.go +++ b/release/releaser_test.go @@ -345,14 +345,16 @@ func Test_FilterLogic(t *testing.T) { }, }, } { - checkout, cleanup := setup(t) - defer cleanup() - testRelease(t, tst.Name, &ReleaseContext{ - cluster: cluster, - manifests: mockManifests, - registry: mockRegistry, - repo: checkout, - }, tst.Spec, tst.Expected) + t.Run(tst.Name, func(t *testing.T) { + checkout, cleanup := setup(t) + defer cleanup() + testRelease(t, &ReleaseContext{ + cluster: cluster, + manifests: mockManifests, + registry: mockRegistry, + repo: checkout, + }, tst.Spec, tst.Expected) + }) } } @@ -413,19 +415,21 @@ func Test_ImageStatus(t *testing.T) { }, }, } { - checkout, cleanup := setup(t) - defer cleanup() - ctx := &ReleaseContext{ - cluster: cluster, - manifests: mockManifests, - repo: checkout, - registry: upToDateRegistry, - } - testRelease(t, tst.Name, ctx, tst.Spec, tst.Expected) + t.Run(tst.Name, func(t *testing.T) { + checkout, cleanup := setup(t) + defer cleanup() + ctx := &ReleaseContext{ + cluster: cluster, + manifests: mockManifests, + repo: checkout, + registry: upToDateRegistry, + } + testRelease(t, ctx, tst.Spec, tst.Expected) + }) } } -func testRelease(t *testing.T, name string, ctx *ReleaseContext, spec update.ReleaseSpec, expected update.Result) { +func testRelease(t *testing.T, ctx *ReleaseContext, spec update.ReleaseSpec, expected update.Result) { results, err := Release(ctx, spec, log.NewNopLogger()) if err != nil { t.Fatal(err) @@ -433,7 +437,7 @@ func testRelease(t *testing.T, name string, ctx *ReleaseContext, spec update.Rel if !reflect.DeepEqual(expected, results) { exp, _ := json.Marshal(expected) got, _ := json.Marshal(results) - t.Errorf("%s\n--- expected ---\n%s\n--- got ---\n%s\n", name, string(exp), string(got)) + t.Errorf("--- expected ---\n%s\n--- got ---\n%s\n", string(exp), string(got)) } } From 52eb5a5c3e77e13ffcb1d20317277b82e7d4607c Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Tue, 12 Jun 2018 16:15:09 +0100 Subject: [PATCH 3/3] Write whole file updates at a time Up to this point, the update (and release) packages assumed that each file they were dealing with corresponded to a single manifest, because we couldn't deal with updating files with multiple manifests in them. Since we can now update a resource in a multidoc file or List, we can do this correctly and more simply by just overwriting each file after running its contents through the update process, serially. --- cluster/kubernetes/testfiles/data.go | 75 +++++++++++---- release/context.go | 17 ++-- release/releaser_test.go | 136 +++++++++++++++++++++++---- update/automated.go | 6 -- update/release.go | 6 -- update/service.go | 11 +-- 6 files changed, 189 insertions(+), 62 deletions(-) diff --git a/cluster/kubernetes/testfiles/data.go b/cluster/kubernetes/testfiles/data.go index 322be2317..2ec9eb288 100644 --- a/cluster/kubernetes/testfiles/data.go +++ b/cluster/kubernetes/testfiles/data.go @@ -45,11 +45,13 @@ func WriteTestFiles(dir string) error { // ResourceMap is the map of resource names to relative paths, which // must correspond with `Files` below. var ResourceMap = map[flux.ResourceID]string{ - flux.MustParseResourceID("default:deployment/helloworld"): "helloworld-deploy.yaml", - flux.MustParseResourceID("default:deployment/locked-service"): "locked-service-deploy.yaml", - flux.MustParseResourceID("default:deployment/test-service"): "test/test-service-deploy.yaml", - flux.MustParseResourceID("default:deployment/www-example-io"): "multi.yaml", - flux.MustParseResourceID("default:service/www-example-service"): "multi.yaml", + flux.MustParseResourceID("default:deployment/helloworld"): "helloworld-deploy.yaml", + flux.MustParseResourceID("default:deployment/locked-service"): "locked-service-deploy.yaml", + flux.MustParseResourceID("default:deployment/test-service"): "test/test-service-deploy.yaml", + flux.MustParseResourceID("default:deployment/multi-deploy"): "multi.yaml", + flux.MustParseResourceID("default:service/multi-service"): "multi.yaml", + flux.MustParseResourceID("default:deployment/list-deploy"): "list.yaml", + flux.MustParseResourceID("default:service/list-service"): "list.yaml", } // ServiceMap ... given a base path, construct the map representing @@ -60,14 +62,16 @@ func ServiceMap(dir string) map[flux.ResourceID][]string { flux.MustParseResourceID("default:deployment/helloworld"): []string{filepath.Join(dir, "helloworld-deploy.yaml")}, flux.MustParseResourceID("default:deployment/locked-service"): []string{filepath.Join(dir, "locked-service-deploy.yaml")}, flux.MustParseResourceID("default:deployment/test-service"): []string{filepath.Join(dir, "test/test-service-deploy.yaml")}, - flux.MustParseResourceID("default:deployment/www-example-io"): []string{filepath.Join(dir, "multi.yaml")}, + flux.MustParseResourceID("default:deployment/multi-deploy"): []string{filepath.Join(dir, "multi.yaml")}, + flux.MustParseResourceID("default:deployment/list-deploy"): []string{filepath.Join(dir, "list.yaml")}, } } var Files = map[string]string{ "garbage": "This should just be ignored, since it is not YAML", // Some genuine manifests - "helloworld-deploy.yaml": `apiVersion: extensions/v1beta1 + "helloworld-deploy.yaml": `--- +apiVersion: extensions/v1beta1 kind: Deployment metadata: name: helloworld @@ -136,42 +140,75 @@ spec: - containerPort: 80 `, // A multidoc, since we support those now - "multi.yaml": `apiVersion: apps/v1beta1 + "multi.yaml": `--- +apiVersion: apps/v1beta1 kind: Deployment metadata: annotations: flux.weave.works/automated: "true" - name: www-example-io + name: multi-deploy spec: replicas: 1 template: metadata: labels: - app : www-example-io + app : multi-app spec: containers: - - name: www-example-io + - name: hello image: quay.io/weaveworks/helloworld:master-a000001 imagePullPolicy: Always ports: - containerPort: 80 - imagePullSecrets: - - name: imagesecret --- apiVersion: v1 kind: Service metadata: - labels: - app: www-example-io - name: www-example-service + name: multi-service spec: type: NodePort ports: - - name: www-example-io - port: 80 + - port: 80 protocol: TCP selector: - app: www-example-io + app: multi-app +`, + + // A List resource + "list.yaml": `--- +apiVersion: v1 +kind: List +items: +- apiVersion: apps/v1beta1 + kind: Deployment + metadata: + name: list-deploy + spec: + replicas: 1 + template: + metadata: + labels: + app : list-app + spec: + containers: + - name: hello + image: quay.io/weaveworks/helloworld:master-a000001 + imagePullPolicy: Always + ports: + - containerPort: 80 +- apiVersion: v1 + kind: Service + metadata: + labels: + app: list-app + name: list-service + spec: + type: NodePort + ports: + - port: 80 + protocol: TCP + selector: + app: list-app `, // A tricksy chart directory, which should be skipped entirely. Adapted from diff --git a/release/context.go b/release/context.go index fa5953f51..e3282e779 100644 --- a/release/context.go +++ b/release/context.go @@ -42,11 +42,17 @@ func (rc *ReleaseContext) Manifests() cluster.Manifests { func (rc *ReleaseContext) WriteUpdates(updates []*update.ControllerUpdate) error { err := func() error { for _, update := range updates { - fi, err := os.Stat(update.ManifestPath) + manifestBytes, err := ioutil.ReadFile(update.ManifestPath) if err != nil { return err } - if err = ioutil.WriteFile(update.ManifestPath, update.ManifestBytes, fi.Mode()); err != nil { + for _, container := range update.Updates { + manifestBytes, err = rc.manifests.UpdateImage(manifestBytes, update.ResourceID, container.Container, container.Target) + if err != nil { + return err + } + } + if err = ioutil.WriteFile(update.ManifestPath, manifestBytes, os.FileMode(0600)); err != nil { return err } } @@ -129,10 +135,9 @@ func (rc *ReleaseContext) WorkloadsForUpdate() (map[flux.ResourceID]*update.Cont for _, res := range resources { if wl, ok := res.(resource.Workload); ok { defined[res.ResourceID()] = &update.ControllerUpdate{ - ResourceID: res.ResourceID(), - Resource: wl, - ManifestPath: filepath.Join(rc.repo.Dir(), res.Source()), - ManifestBytes: res.Bytes(), + ResourceID: res.ResourceID(), + Resource: wl, + ManifestPath: filepath.Join(rc.repo.Dir(), res.Source()), } } } diff --git a/release/releaser_test.go b/release/releaser_test.go index 4664ac596..5fc2f2718 100644 --- a/release/releaser_test.go +++ b/release/releaser_test.go @@ -196,7 +196,8 @@ func Test_FilterLogic(t *testing.T) { }, flux.MustParseResourceID("default:deployment/locked-service"): ignoredNotIncluded, flux.MustParseResourceID("default:deployment/test-service"): ignoredNotIncluded, - flux.MustParseResourceID("default:deployment/www-example-io"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/multi-deploy"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/list-deploy"): ignoredNotIncluded, }, }, { Name: "exclude specific service", @@ -226,8 +227,9 @@ func Test_FilterLogic(t *testing.T) { Status: update.ReleaseStatusIgnored, Error: update.Excluded, }, - flux.MustParseResourceID("default:deployment/test-service"): skippedNotInCluster, - flux.MustParseResourceID("default:deployment/www-example-io"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/test-service"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/multi-deploy"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/list-deploy"): skippedNotInCluster, }, }, { Name: "update specific image", @@ -252,14 +254,9 @@ func Test_FilterLogic(t *testing.T) { Status: update.ReleaseStatusIgnored, Error: update.DifferentImage, }, - flux.MustParseResourceID("default:deployment/test-service"): update.ControllerResult{ - Status: update.ReleaseStatusSkipped, - Error: update.NotInCluster, - }, - flux.MustParseResourceID("default:deployment/www-example-io"): update.ControllerResult{ - Status: update.ReleaseStatusSkipped, - Error: update.NotInCluster, - }, + flux.MustParseResourceID("default:deployment/test-service"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/multi-deploy"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/list-deploy"): skippedNotInCluster, }, }, // skipped if: not ignored AND (locked or not found in cluster) @@ -292,8 +289,9 @@ func Test_FilterLogic(t *testing.T) { Status: update.ReleaseStatusSkipped, Error: update.Locked, }, - flux.MustParseResourceID("default:deployment/test-service"): skippedNotInCluster, - flux.MustParseResourceID("default:deployment/www-example-io"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/test-service"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/multi-deploy"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/list-deploy"): skippedNotInCluster, }, }, { @@ -324,8 +322,9 @@ func Test_FilterLogic(t *testing.T) { Status: update.ReleaseStatusSkipped, Error: update.Locked, }, - flux.MustParseResourceID("default:deployment/test-service"): skippedNotInCluster, - flux.MustParseResourceID("default:deployment/www-example-io"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/test-service"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/multi-deploy"): skippedNotInCluster, + flux.MustParseResourceID("default:deployment/list-deploy"): skippedNotInCluster, }, }, { @@ -340,7 +339,8 @@ func Test_FilterLogic(t *testing.T) { flux.MustParseResourceID("default:deployment/helloworld"): ignoredNotIncluded, flux.MustParseResourceID("default:deployment/locked-service"): ignoredNotIncluded, flux.MustParseResourceID("default:deployment/test-service"): ignoredNotIncluded, - flux.MustParseResourceID("default:deployment/www-example-io"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/multi-deploy"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/list-deploy"): ignoredNotIncluded, flux.MustParseResourceID(notInRepoService): skippedNotInRepo, }, }, @@ -390,7 +390,8 @@ func Test_ImageStatus(t *testing.T) { Expected: update.Result{ flux.MustParseResourceID("default:deployment/helloworld"): ignoredNotIncluded, flux.MustParseResourceID("default:deployment/locked-service"): ignoredNotIncluded, - flux.MustParseResourceID("default:deployment/www-example-io"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/multi-deploy"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/list-deploy"): ignoredNotIncluded, flux.MustParseResourceID("default:deployment/test-service"): update.ControllerResult{ Status: update.ReleaseStatusIgnored, Error: update.DoesNotUseImage, @@ -411,7 +412,8 @@ func Test_ImageStatus(t *testing.T) { }, flux.MustParseResourceID("default:deployment/locked-service"): ignoredNotIncluded, flux.MustParseResourceID("default:deployment/test-service"): ignoredNotIncluded, - flux.MustParseResourceID("default:deployment/www-example-io"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/multi-deploy"): ignoredNotIncluded, + flux.MustParseResourceID("default:deployment/list-deploy"): ignoredNotIncluded, }, }, } { @@ -429,6 +431,102 @@ func Test_ImageStatus(t *testing.T) { } } +func Test_UpdateMultidoc(t *testing.T) { + egID := flux.MustParseResourceID("default:deployment/multi-deploy") + egSvc := cluster.Controller{ + ID: egID, + Containers: cluster.ContainersOrExcuse{ + Containers: []resource.Container{ + { + Name: "hello", + Image: oldRef, + }, + }, + }, + } + + cluster := mockCluster(hwSvc, lockedSvc, egSvc) // no testsvc in cluster, but it _is_ in repo + checkout, cleanup := setup(t) + defer cleanup() + ctx := &ReleaseContext{ + cluster: cluster, + manifests: mockManifests, + repo: checkout, + registry: mockRegistry, + } + spec := update.ReleaseSpec{ + ServiceSpecs: []update.ResourceSpec{"default:deployment/multi-deploy"}, + ImageSpec: update.ImageSpecLatest, + Kind: update.ReleaseKindExecute, + } + results, err := Release(ctx, spec, log.NewNopLogger()) + if err != nil { + t.Error(err) + } + controllerResult, ok := results[egID] + if !ok { + t.Fatal("controller not found after update") + } + if !reflect.DeepEqual(update.ControllerResult{ + Status: update.ReleaseStatusSuccess, + PerContainer: []update.ContainerUpdate{{ + Container: "hello", + Current: oldRef, + Target: newHwRef, + }}, + }, controllerResult) { + t.Errorf("did not get expected controller result (see test code), got %#v", controllerResult) + } +} + +func Test_UpdateList(t *testing.T) { + egID := flux.MustParseResourceID("default:deployment/list-deploy") + egSvc := cluster.Controller{ + ID: egID, + Containers: cluster.ContainersOrExcuse{ + Containers: []resource.Container{ + { + Name: "hello", + Image: oldRef, + }, + }, + }, + } + + cluster := mockCluster(hwSvc, lockedSvc, egSvc) // no testsvc in cluster, but it _is_ in repo + checkout, cleanup := setup(t) + defer cleanup() + ctx := &ReleaseContext{ + cluster: cluster, + manifests: mockManifests, + repo: checkout, + registry: mockRegistry, + } + spec := update.ReleaseSpec{ + ServiceSpecs: []update.ResourceSpec{"default:deployment/list-deploy"}, + ImageSpec: update.ImageSpecLatest, + Kind: update.ReleaseKindExecute, + } + results, err := Release(ctx, spec, log.NewNopLogger()) + if err != nil { + t.Error(err) + } + controllerResult, ok := results[egID] + if !ok { + t.Fatal("controller not found after update") + } + if !reflect.DeepEqual(update.ControllerResult{ + Status: update.ReleaseStatusSuccess, + PerContainer: []update.ContainerUpdate{{ + Container: "hello", + Current: oldRef, + Target: newHwRef, + }}, + }, controllerResult) { + t.Errorf("did not get expected controller result (see test code), got %#v", controllerResult) + } +} + func testRelease(t *testing.T, ctx *ReleaseContext, spec update.ReleaseSpec, expected update.Result) { results, err := Release(ctx, spec, log.NewNopLogger()) if err != nil { @@ -452,7 +550,7 @@ func (m *badManifests) UpdateImage(def []byte, resourceID flux.ResourceID, conta return def, nil } -func TestBadRelease(t *testing.T) { +func Test_BadRelease(t *testing.T) { cluster := mockCluster(hwSvc) spec := update.ReleaseSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, diff --git a/update/automated.go b/update/automated.go index c2d8f74ea..294ef049b 100644 --- a/update/automated.go +++ b/update/automated.go @@ -107,12 +107,6 @@ func (a *Automated) calculateImageUpdates(rc ReleaseContext, candidates []*Contr // the format of the image name as it is in the // resource (e.g., to avoid canonicalising it) newImageID := currentImageID.WithNewTag(change.ImageID.Tag) - var err error - u.ManifestBytes, err = rc.Manifests().UpdateImage(u.ManifestBytes, u.ResourceID, container.Name, newImageID) - if err != nil { - return nil, err - } - containerUpdates = append(containerUpdates, ContainerUpdate{ Container: container.Name, Current: currentImageID, diff --git a/update/release.go b/update/release.go index df52bc574..ff5a23c50 100644 --- a/update/release.go +++ b/update/release.go @@ -251,12 +251,6 @@ func (s ReleaseSpec) calculateImageUpdates(rc ReleaseContext, candidates []*Cont // appears in the manifest, whereas what we have is the // canonical form. newImageID := currentImageID.WithNewTag(latestImage.ID.Tag) - - u.ManifestBytes, err = rc.Manifests().UpdateImage(u.ManifestBytes, u.ResourceID, container.Name, newImageID) - if err != nil { - return nil, err - } - containerUpdates = append(containerUpdates, ContainerUpdate{ Container: container.Name, Current: currentImageID, diff --git a/update/service.go b/update/service.go index 2807da511..073d8b46e 100644 --- a/update/service.go +++ b/update/service.go @@ -7,12 +7,11 @@ import ( ) type ControllerUpdate struct { - ResourceID flux.ResourceID - Controller cluster.Controller - Resource resource.Workload - ManifestPath string - ManifestBytes []byte - Updates []ContainerUpdate + ResourceID flux.ResourceID + Controller cluster.Controller + Resource resource.Workload + ManifestPath string + Updates []ContainerUpdate } type ControllerFilter interface {