Skip to content

Commit

Permalink
support ignoring certain fields to avoid unnecessary fallback to diff…
Browse files Browse the repository at this point in the history
…ing live state

primary example that drove this change: Deployment resource has an annotation that track its revision. To avoid race between kapp capturing what has been defaulted on the object and cluster adding revision annotation, kapp includes a default rule to ignore that field when diffing against last applied content.
  • Loading branch information
cppforlife committed Sep 21, 2019
1 parent a41d17a commit 6762ac7
Show file tree
Hide file tree
Showing 12 changed files with 285 additions and 34 deletions.
9 changes: 9 additions & 0 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ templateRules:
additionalLabels:
department: marketing
cost-center: mar201

diffAgainstLastAppliedFieldExclusionRules:
- path: [metadata, annotations, "deployment.kubernetes.io/revision"]
resourceMatchers:
- apiVersionKindMatcher:
apiVersion: apps/v1
kind: Deployment
```
`rebaseRules` specify origin of field values. Kubernetes cluster generates (or defaults) some field values, hence these values will need to be merged in future to avoid flagging them during diffing. Common example is `v1/Service`'s `spec.clusterIP` field is automatically populated if it's not set. See [HPA and Deployment rebase](hpa-deployment-rebase.md) example.
Expand All @@ -56,6 +63,8 @@ additionalLabels:

`additionalLabels` specify additional labels to apply to all resources for custom uses by the user (added based on `ownershipLabelRules`).

`diffAgainstLastAppliedFieldExclusionRules` specify which fields should be removed before diff-ing against last applied resource. These rules are useful for fields are "owned" by the cluster/controllers, and are only later updated. For example `Deployment` resource has an annotation that gets set after a little bit of time after resource is created/updated (not during resource admission). It's typically not necessary to use this configuration.

### Resource matchers

Resource matchers (as used by `rebaseRules` and `ownershipLabelRules`):
Expand Down
2 changes: 1 addition & 1 deletion pkg/kapp/cmd/app/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (o *DeleteOptions) Run() error {
}

existingResources = applicableExistingResources
changeFactory := ctldiff.NewChangeFactory(nil)
changeFactory := ctldiff.NewChangeFactory(nil, nil)

o.changeIgnored(existingResources)

Expand Down
2 changes: 1 addition & 1 deletion pkg/kapp/cmd/app/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (o *DeployOptions) Run() error {
}

existingResources = resourceFilter.Apply(existingResources)
changeFactory := ctldiff.NewChangeFactory(conf.RebaseMods())
changeFactory := ctldiff.NewChangeFactory(conf.RebaseMods(), conf.DiffAgainstLastAppliedFieldExclusionMods())

changes, err := ctldiff.NewChangeSetWithTemplates(
existingResources, newResources, conf.TemplateRules(),
Expand Down
2 changes: 1 addition & 1 deletion pkg/kapp/cmd/app/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (o *InspectOptions) Run() error {
switch {
case o.Raw:
for _, res := range resources {
historylessRes, err := ctldiff.NewResourceWithHistory(res, nil).HistorylessResource()
historylessRes, err := ctldiff.NewResourceWithHistory(res, nil, nil).HistorylessResource()
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kapp/cmd/tools/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (o *DiffOptions) Run() error {
return err
}

changeFactory := ctldiff.NewChangeFactory(nil)
changeFactory := ctldiff.NewChangeFactory(nil, nil)

changes, err := ctldiff.NewChangeSet(existingResources, newResources, o.DiffFlags.ChangeSetOpts, changeFactory).Calculate()
if err != nil {
Expand Down
10 changes: 10 additions & 0 deletions pkg/kapp/config/conf.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ func (c Conf) RebaseMods() []ctlres.ResourceModWithMultiple {
return mods
}

func (c Conf) DiffAgainstLastAppliedFieldExclusionMods() []ctlres.FieldRemoveMod {
var mods []ctlres.FieldRemoveMod
for _, config := range c.configs {
for _, rule := range config.DiffAgainstLastAppliedFieldExclusionRules {
mods = append(mods, rule.AsMods()...)
}
}
return mods
}

func (c Conf) OwnershipLabelMods() func(kvs map[string]string) []ctlres.StringMapAppendMod {
return func(kvs map[string]string) []ctlres.StringMapAppendMod {
var mods []ctlres.StringMapAppendMod
Expand Down
19 changes: 18 additions & 1 deletion pkg/kapp/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ type Config struct {
LabelScopingRules []LabelScopingRule
TemplateRules []TemplateRule

AdditionalLabels map[string]string
AdditionalLabels map[string]string
DiffAgainstLastAppliedFieldExclusionRules []DiffAgainstLastAppliedFieldExclusionRule
}

type RebaseRule struct {
Expand All @@ -31,6 +32,11 @@ type RebaseRule struct {
Sources []ctlres.FieldCopyModSource
}

type DiffAgainstLastAppliedFieldExclusionRule struct {
ResourceMatchers []ResourceMatcher
Path ctlres.Path
}

type OwnershipLabelRule struct {
ResourceMatchers []ResourceMatcher
Path ctlres.Path
Expand Down Expand Up @@ -119,6 +125,17 @@ func (r RebaseRule) AsMods() []ctlres.ResourceModWithMultiple {
return mods
}

func (r DiffAgainstLastAppliedFieldExclusionRule) AsMods() []ctlres.FieldRemoveMod {
var mods []ctlres.FieldRemoveMod
for _, matcher := range r.ResourceMatchers {
mods = append(mods, ctlres.FieldRemoveMod{
ResourceMatcher: matcher.AsResourceMatcher(),
Path: r.Path,
})
}
return mods
}

func (r OwnershipLabelRule) AsMods(kvs map[string]string) []ctlres.StringMapAppendMod {
return stringMapAppendRule{ResourceMatchers: r.ResourceMatchers, Path: r.Path}.AsMods(kvs)
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/kapp/config/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,16 @@ rebaseRules:
- path: [metadata, annotations, "deployment.kubernetes.io/revision"]
type: copy
sources: [new, existing]
resourceMatchers: &recentAppsDeployments
resourceMatchers: &builtinAppsDeploymentWithRevAnnKey
- apiVersionKindMatcher: {apiVersion: apps/v1, kind: Deployment}
- apiVersionKindMatcher: {apiVersion: apps/v1beta1, kind: Deployment}
- apiVersionKindMatcher: {apiVersion: apps/v1beta2, kind: Deployment}
- apiVersionKindMatcher: {apiVersion: extensions/v1beta1, kind: Deployment}
diffAgainstLastAppliedFieldExclusionRules:
- path: [metadata, annotations, "deployment.kubernetes.io/revision"]
resourceMatchers: *builtinAppsDeploymentWithRevAnnKey
ownershipLabelRules:
- path: [metadata, labels]
resourceMatchers:
Expand Down
17 changes: 10 additions & 7 deletions pkg/kapp/diff/change_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,21 @@ import (
)

type ChangeFactory struct {
mods []ctlres.ResourceModWithMultiple
rebaseMods []ctlres.ResourceModWithMultiple
diffAgainstLastAppliedFieldExclusionMods []ctlres.FieldRemoveMod
}

func NewChangeFactory(mods []ctlres.ResourceModWithMultiple) ChangeFactory {
return ChangeFactory{mods}
func NewChangeFactory(rebaseMods []ctlres.ResourceModWithMultiple,
diffAgainstLastAppliedFieldExclusionMods []ctlres.FieldRemoveMod) ChangeFactory {

return ChangeFactory{rebaseMods, diffAgainstLastAppliedFieldExclusionMods}
}

func (f ChangeFactory) NewChangeAgainstLastApplied(existingRes, newRes ctlres.Resource) (Change, error) {
if existingRes != nil {
lastAppliedRes := f.NewResourceWithHistory(existingRes).LastAppliedResource()
if lastAppliedRes != nil {
rebasedLastAppliedRes, err := NewRebasedResource(existingRes, lastAppliedRes, f.mods).Resource()
rebasedLastAppliedRes, err := NewRebasedResource(existingRes, lastAppliedRes, f.rebaseMods).Resource()
if err != nil {
return nil, err
}
Expand All @@ -40,7 +43,7 @@ func (f ChangeFactory) NewChangeAgainstLastApplied(existingRes, newRes ctlres.Re
newRes = historylessNewRes
}

rebasedNewRes, err := NewRebasedResource(existingRes, newRes, f.mods).Resource()
rebasedNewRes, err := NewRebasedResource(existingRes, newRes, f.rebaseMods).Resource()
if err != nil {
return nil, err
}
Expand All @@ -67,7 +70,7 @@ func (f ChangeFactory) NewExactChange(existingRes, newRes ctlres.Resource) (Chan
newRes = historylessNewRes
}

rebasedNewRes, err := NewRebasedResource(existingRes, newRes, f.mods).Resource()
rebasedNewRes, err := NewRebasedResource(existingRes, newRes, f.rebaseMods).Resource()
if err != nil {
return nil, err
}
Expand All @@ -76,5 +79,5 @@ func (f ChangeFactory) NewExactChange(existingRes, newRes ctlres.Resource) (Chan
}

func (f ChangeFactory) NewResourceWithHistory(resource ctlres.Resource) ResourceWithHistory {
return NewResourceWithHistory(resource, &f)
return NewResourceWithHistory(resource, &f, f.diffAgainstLastAppliedFieldExclusionMods)
}
156 changes: 152 additions & 4 deletions pkg/kapp/diff/change_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
ctlres "github.com/k14s/kapp/pkg/kapp/resources"
)

func TestChangeSetRebaseWithoutNewButWithUnexpectedChanges(t *testing.T) {
func TestChangeSet_RebaseWithoutNew_And_WithUnexpectedChanges(t *testing.T) {
newRes := ctlres.MustNewResourceFromBytes([]byte(`
metadata:
name: my-res
Expand Down Expand Up @@ -44,7 +44,7 @@ metadata:
},
}

changeFactory := ctldiff.NewChangeFactory(mods)
changeFactory := ctldiff.NewChangeFactory(mods, nil)
changeSet := ctldiff.NewChangeSet([]ctlres.Resource{existingRes}, []ctlres.Resource{newRes},
ctldiff.ChangeSetOpts{}, changeFactory)

Expand All @@ -68,7 +68,7 @@ metadata:
}
}

func TestChangeSetRebaseWithNewAndUnexpectedChanges(t *testing.T) {
func TestChangeSet_RebaseWithNew_And_UnexpectedChanges(t *testing.T) {
newRes := ctlres.MustNewResourceFromBytes([]byte(`
metadata:
name: my-res
Expand Down Expand Up @@ -106,7 +106,7 @@ metadata:
},
}

changeFactory := ctldiff.NewChangeFactory(mods)
changeFactory := ctldiff.NewChangeFactory(mods, nil)
changeSet := ctldiff.NewChangeSet([]ctlres.Resource{existingRes}, []ctlres.Resource{newRes},
ctldiff.ChangeSetOpts{}, changeFactory)

Expand All @@ -130,3 +130,151 @@ metadata:
t.Fatalf("Expected diff to match: actual >>>%s<<< vs expected >>>%s<<< %d %d", actualDiff, expectedDiff, len(actualDiff), len(expectedDiff))
}
}

func TestChangeSet_WithoutNew_And_WithoutUnexpectedChanges_And_IgnoredFields(t *testing.T) {
newRes := ctlres.MustNewResourceFromBytes([]byte(`
metadata:
name: my-res
`))

existingRes := ctlres.MustNewResourceFromBytes([]byte(`
metadata:
name: my-res
annotations:
unexpected-ignored: "1"
rebased: "1"
kapp.k14s.io/original: |
metadata:
name: my-res
kapp.k14s.io/original-diff-md5: "58e0494c51d30eb3494f7c9198986bb9"
`))

rebaseMods := []ctlres.ResourceModWithMultiple{
ctlres.FieldCopyMod{
ResourceMatcher: ctlres.AllResourceMatcher{},
Path: ctlres.NewPathFromStrings([]string{"metadata"}),
Sources: []ctlres.FieldCopyModSource{ctlres.FieldCopyModSourceExisting},
},
ctlres.FieldRemoveMod{
ResourceMatcher: ctlres.AllResourceMatcher{},
Path: ctlres.NewPathFromStrings([]string{"metadata", "annotations"}),
},
ctlres.FieldCopyMod{
ResourceMatcher: ctlres.AllResourceMatcher{},
Path: ctlres.NewPathFromStrings([]string{"metadata", "annotations"}),
Sources: []ctlres.FieldCopyModSource{ctlres.FieldCopyModSourceNew},
},
ctlres.FieldCopyMod{
ResourceMatcher: ctlres.AllResourceMatcher{},
Path: ctlres.NewPathFromStrings([]string{"metadata", "annotations", "rebased"}),
Sources: []ctlres.FieldCopyModSource{ctlres.FieldCopyModSourceNew, ctlres.FieldCopyModSourceExisting},
},
}

ignoreFieldsMods := []ctlres.FieldRemoveMod{
ctlres.FieldRemoveMod{
ResourceMatcher: ctlres.AllResourceMatcher{},
Path: ctlres.NewPathFromStrings([]string{"metadata", "annotations", "unexpected-ignored"}),
},
}

changeFactory := ctldiff.NewChangeFactory(rebaseMods, ignoreFieldsMods)
changeSet := ctldiff.NewChangeSet([]ctlres.Resource{existingRes}, []ctlres.Resource{newRes},
ctldiff.ChangeSetOpts{AgainstLastApplied: true}, changeFactory)

changes, err := changeSet.Calculate()
if err != nil {
t.Fatalf("Expected non-err")
}

actualDiff := changes[0].TextDiff().FullString()

expectedDiff := strings.Replace(` 0, 0 metadata:
1, 1 annotations:
2, 2 rebased: "1"
3, 3 name: my-res
4, 4 <---space
`, "<---space", "", -1)

if actualDiff != expectedDiff {
t.Fatalf("Expected diff to match: actual >>>%s<<< vs expected >>>%s<<< %d %d", actualDiff, expectedDiff, len(actualDiff), len(expectedDiff))
}
}

func TestChangeSet_WithoutNew_And_WithUnexpectedChanges_And_IgnoredFields(t *testing.T) {
newRes := ctlres.MustNewResourceFromBytes([]byte(`
metadata:
name: my-res
`))

existingRes := ctlres.MustNewResourceFromBytes([]byte(`
metadata:
name: my-res
annotations:
unexpected: "1"
rebased-ignored: "1"
rebased: "1"
kapp.k14s.io/original: |
metadata:
name: my-res
kapp.k14s.io/original-diff-md5: "58e0494c51d30eb3494f7c9198986bb9"
`))

rebaseMods := []ctlres.ResourceModWithMultiple{
ctlres.FieldCopyMod{
ResourceMatcher: ctlres.AllResourceMatcher{},
Path: ctlres.NewPathFromStrings([]string{"metadata"}),
Sources: []ctlres.FieldCopyModSource{ctlres.FieldCopyModSourceExisting},
},
ctlres.FieldRemoveMod{
ResourceMatcher: ctlres.AllResourceMatcher{},
Path: ctlres.NewPathFromStrings([]string{"metadata", "annotations"}),
},
ctlres.FieldCopyMod{
ResourceMatcher: ctlres.AllResourceMatcher{},
Path: ctlres.NewPathFromStrings([]string{"metadata", "annotations"}),
Sources: []ctlres.FieldCopyModSource{ctlres.FieldCopyModSourceNew},
},
ctlres.FieldCopyMod{
ResourceMatcher: ctlres.AllResourceMatcher{},
Path: ctlres.NewPathFromStrings([]string{"metadata", "annotations", "rebased"}),
Sources: []ctlres.FieldCopyModSource{ctlres.FieldCopyModSourceNew, ctlres.FieldCopyModSourceExisting},
},
ctlres.FieldCopyMod{
ResourceMatcher: ctlres.AllResourceMatcher{},
Path: ctlres.NewPathFromStrings([]string{"metadata", "annotations", "rebased-ignored"}),
Sources: []ctlres.FieldCopyModSource{ctlres.FieldCopyModSourceNew, ctlres.FieldCopyModSourceExisting},
},
}

ignoreFieldsMods := []ctlres.FieldRemoveMod{
ctlres.FieldRemoveMod{
ResourceMatcher: ctlres.AllResourceMatcher{},
Path: ctlres.NewPathFromStrings([]string{"metadata", "annotations", "rebased-ignored"}),
},
}

changeFactory := ctldiff.NewChangeFactory(rebaseMods, ignoreFieldsMods)
changeSet := ctldiff.NewChangeSet([]ctlres.Resource{existingRes}, []ctlres.Resource{newRes},
ctldiff.ChangeSetOpts{AgainstLastApplied: true}, changeFactory)

changes, err := changeSet.Calculate()
if err != nil {
t.Fatalf("Expected non-err")
}

actualDiff := changes[0].TextDiff().FullString()

expectedDiff := strings.Replace(` 0, 0 metadata:
1, 1 annotations:
2, 2 rebased: "1"
3, 3 rebased-ignored: "1"
4, 4 - unexpected: "1"
5, 4 name: my-res
6, 5 <---space
`, "<---space", "", -1)

if actualDiff != expectedDiff {
t.Fatalf("Expected diff to match: actual >>>%s<<< vs expected >>>%s<<< %d %d", actualDiff, expectedDiff, len(actualDiff), len(expectedDiff))
}
}
Loading

0 comments on commit 6762ac7

Please sign in to comment.