Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Write whole file updates at a time
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@squaremo
squaremo committed Jun 12, 2018
1 parent e9a95a3 commit 52eb5a5
Show file tree
Hide file tree
Showing 6 changed files with 189 additions and 62 deletions.
75 changes: 56 additions & 19 deletions cluster/kubernetes/testfiles/data.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
17 changes: 11 additions & 6 deletions release/context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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()),
}
}
}
Expand Down
136 changes: 117 additions & 19 deletions release/releaser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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)
Expand Down Expand Up @@ -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,
},
},
{
Expand Down Expand Up @@ -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,
},
},
{
Expand All @@ -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,
},
},
Expand Down Expand Up @@ -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,
Expand All @@ -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,
},
},
} {
Expand All @@ -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 {
Expand All @@ -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},
Expand Down
6 changes: 0 additions & 6 deletions update/automated.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 0 additions & 6 deletions update/release.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 52eb5a5

Please sign in to comment.