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

Refactor update and release packages to break some dependencies #1033

Closed
wants to merge 1 commit into from
Closed
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
8 changes: 4 additions & 4 deletions cluster/kubernetes/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@ import (
"github.com/weaveworks/flux/policy"
)

func (m *Manifests) UpdatePolicies(in []byte, update policy.Update) ([]byte, error) {
tagAll, _ := update.Add.Get(policy.TagAll)
func (m *Manifests) UpdatePolicies(in []byte, add policy.Set, remove policy.Set) ([]byte, error) {
tagAll, _ := add.Get(policy.TagAll)
return updateAnnotations(in, tagAll, func(a map[string]string) map[string]string {
for p, v := range update.Add {
for p, v := range add {
if p == policy.TagAll {
continue
}
a[resource.PolicyPrefix+string(p)] = v
}
for p, _ := range update.Remove {
for p, _ := range remove {
delete(a, resource.PolicyPrefix+string(p))
}
return a
Expand Down
29 changes: 15 additions & 14 deletions cluster/kubernetes/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,51 +8,52 @@ import (
"text/template"

"github.com/weaveworks/flux/policy"
"github.com/weaveworks/flux/update"
)

func TestUpdatePolicies(t *testing.T) {
for _, c := range []struct {
name string
in, out map[string]string
update policy.Update
change update.PolicyChange
}{
{
name: "adding annotation with others existing",
in: map[string]string{"prometheus.io.scrape": "false"},
out: map[string]string{"flux.weave.works/automated": "true", "prometheus.io.scrape": "false"},
update: policy.Update{
change: update.PolicyChange{
Add: policy.Set{policy.Automated: "true"},
},
},
{
name: "adding annotation when already has annotation",
in: map[string]string{"flux.weave.works/automated": "true"},
out: map[string]string{"flux.weave.works/automated": "true"},
update: policy.Update{
change: update.PolicyChange{
Add: policy.Set{policy.Automated: "true"},
},
},
{
name: "adding annotation when already has annotation and others",
in: map[string]string{"flux.weave.works/automated": "true", "prometheus.io.scrape": "false"},
out: map[string]string{"flux.weave.works/automated": "true", "prometheus.io.scrape": "false"},
update: policy.Update{
change: update.PolicyChange{
Add: policy.Set{policy.Automated: "true"},
},
},
{
name: "adding first annotation",
in: nil,
out: map[string]string{"flux.weave.works/automated": "true"},
update: policy.Update{
change: update.PolicyChange{
Add: policy.Set{policy.Automated: "true"},
},
},
{
name: "add and remove different annotations at the same time",
in: map[string]string{"flux.weave.works/automated": "true", "prometheus.io.scrape": "false"},
out: map[string]string{"flux.weave.works/locked": "true", "prometheus.io.scrape": "false"},
update: policy.Update{
change: update.PolicyChange{
Add: policy.Set{policy.Locked: "true"},
Remove: policy.Set{policy.Automated: "true"},
},
Expand All @@ -61,7 +62,7 @@ func TestUpdatePolicies(t *testing.T) {
name: "remove overrides add for same key",
in: nil,
out: nil,
update: policy.Update{
change: update.PolicyChange{
Add: policy.Set{policy.Locked: "true"},
Remove: policy.Set{policy.Locked: "true"},
},
Expand All @@ -70,54 +71,54 @@ func TestUpdatePolicies(t *testing.T) {
name: "remove annotation with others existing",
in: map[string]string{"flux.weave.works/automated": "true", "prometheus.io.scrape": "false"},
out: map[string]string{"prometheus.io.scrape": "false"},
update: policy.Update{
change: update.PolicyChange{
Remove: policy.Set{policy.Automated: "true"},
},
},
{
name: "remove last annotation",
in: map[string]string{"flux.weave.works/automated": "true"},
out: nil,
update: policy.Update{
change: update.PolicyChange{
Remove: policy.Set{policy.Automated: "true"},
},
},
{
name: "remove annotation with no annotations",
in: nil,
out: nil,
update: policy.Update{
change: update.PolicyChange{
Remove: policy.Set{policy.Automated: "true"},
},
},
{
name: "remove annotation with only others",
in: map[string]string{"prometheus.io.scrape": "false"},
out: map[string]string{"prometheus.io.scrape": "false"},
update: policy.Update{
change: update.PolicyChange{
Remove: policy.Set{policy.Automated: "true"},
},
},
{
name: "multiline",
in: map[string]string{"flux.weave.works/locked_msg": "|-\n first\n second"},
out: nil,
update: policy.Update{
change: update.PolicyChange{
Remove: policy.Set{policy.LockedMsg: "foo"},
},
},
{
name: "multiline with empty line",
in: map[string]string{"flux.weave.works/locked_msg": "|-\n first\n\n third"},
out: nil,
update: policy.Update{
change: update.PolicyChange{
Remove: policy.Set{policy.LockedMsg: "foo"},
},
},
} {
caseIn := templToString(t, annotationsTemplate, c.in)
caseOut := templToString(t, annotationsTemplate, c.out)
out, err := (&Manifests{}).UpdatePolicies([]byte(caseIn), c.update)
out, err := (&Manifests{}).UpdatePolicies([]byte(caseIn), c.change.Add, c.change.Remove)
if err != nil {
t.Errorf("[%s] %v", c.name, err)
} else if string(out) != caseOut {
Expand Down
6 changes: 3 additions & 3 deletions cluster/kubernetes/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import (
"github.com/weaveworks/flux/image"
)

type update struct {
type testCase struct {
name string
containers []string
updatedImage string
caseIn, caseOut string
}

func testUpdate(t *testing.T, u update) {
func testUpdate(t *testing.T, u testCase) {
id, err := image.ParseRef(u.updatedImage)
if err != nil {
t.Fatal(err)
Expand All @@ -39,7 +39,7 @@ func testUpdate(t *testing.T, u update) {
}

func TestUpdates(t *testing.T) {
for _, c := range []update{
for _, c := range []testCase{
{"common case", case1container, case1image, case1, case1out},
{"new version like number", case2container, case2image, case2, case2out},
{"old version like number", case2container, case2reverseImage, case2out, case2},
Expand Down
2 changes: 1 addition & 1 deletion cluster/manifests.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type Manifests interface {
// Parse the manifests given in an exported blob
ParseManifests([]byte) (map[string]resource.Resource, error)
// UpdatePolicies modifies a manifest to apply the policy update specified
UpdatePolicies([]byte, policy.Update) ([]byte, error)
UpdatePolicies(def []byte, add policy.Set, remove policy.Set) ([]byte, error)
// ServicesWithPolicies returns all services with their associated policies
ServicesWithPolicies(path string) (policy.ResourceMap, error)
}
Expand Down
6 changes: 3 additions & 3 deletions cluster/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type Mock struct {
LoadManifestsFunc func(base, first string, rest ...string) (map[string]resource.Resource, error)
ParseManifestsFunc func([]byte) (map[string]resource.Resource, error)
UpdateManifestFunc func(path, resourceID string, f func(def []byte) ([]byte, error)) error
UpdatePoliciesFunc func([]byte, policy.Update) ([]byte, error)
UpdatePoliciesFunc func([]byte, policy.Set, policy.Set) ([]byte, error)
ServicesWithPoliciesFunc func(path string) (policy.ResourceMap, error)
}

Expand Down Expand Up @@ -69,8 +69,8 @@ func (m *Mock) UpdateManifest(path string, resourceID string, f func(def []byte)
return m.UpdateManifestFunc(path, resourceID, f)
}

func (m *Mock) UpdatePolicies(def []byte, p policy.Update) ([]byte, error) {
return m.UpdatePoliciesFunc(def, p)
func (m *Mock) UpdatePolicies(def []byte, add policy.Set, remove policy.Set) ([]byte, error) {
return m.UpdatePoliciesFunc(def, add, remove)
}

func (m *Mock) ServicesWithPolicies(path string) (policy.ResourceMap, error) {
Expand Down
13 changes: 6 additions & 7 deletions cmd/fluxctl/policy_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,10 @@ func (opts *controllerPolicyOpts) RunE(cmd *cobra.Command, args []string) error
}

ctx := context.Background()
updates := policy.Updates{
resourceID: changes,
}
updates := update.Policy{}
updates[resourceID] = changes
jobID, err := opts.API.UpdateManifests(ctx, update.Spec{
Type: update.Policy,
Type: update.SpecPolicy,
Cause: opts.cause,
Spec: updates,
})
Expand All @@ -117,7 +116,7 @@ func (opts *controllerPolicyOpts) RunE(cmd *cobra.Command, args []string) error
return await(ctx, cmd.OutOrStdout(), cmd.OutOrStderr(), opts.API, jobID, false, opts.verbosity)
}

func calculatePolicyChanges(opts *controllerPolicyOpts) (policy.Update, error) {
func calculatePolicyChanges(opts *controllerPolicyOpts) (update.PolicyChange, error) {
add := policy.Set{}
if opts.automate {
add = add.Add(policy.Automated)
Expand Down Expand Up @@ -148,7 +147,7 @@ func calculatePolicyChanges(opts *controllerPolicyOpts) (policy.Update, error) {
for _, tagPair := range opts.tags {
parts := strings.Split(tagPair, "=")
if len(parts) != 2 {
return policy.Update{}, fmt.Errorf("invalid container/tag pair: %q. Expected format is 'container=filter'", tagPair)
return update.PolicyChange{}, fmt.Errorf("invalid container/tag pair: %q. Expected format is 'container=filter'", tagPair)
}

container, tag := parts[0], parts[1]
Expand All @@ -159,7 +158,7 @@ func calculatePolicyChanges(opts *controllerPolicyOpts) (policy.Update, error) {
}
}

return policy.Update{
return update.PolicyChange{
Add: add,
Remove: remove,
}, nil
Expand Down
2 changes: 1 addition & 1 deletion cmd/fluxctl/release_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (opts *controllerReleaseOpts) RunE(cmd *cobra.Command, args []string) error
Excludes: excludes,
}
jobID, err := opts.API.UpdateManifests(ctx, update.Spec{
Type: update.Images,
Type: update.SpecImages,
Cause: opts.cause,
Spec: spec,
})
Expand Down
Loading