-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for server side apply #392
base: develop
Are you sure you want to change the base?
Changes from 13 commits
fd0f790
176ebb5
20bd0a6
27e77f8
6884b1e
7de82bd
1f77748
5ff0cd5
a1b7202
e7b4682
d17a09e
868d362
6a42593
3c0ba71
1e81fce
df4a333
307bbe9
8eb9d07
c11badb
5a5d997
5b6b056
d2a434b
7745d81
9a6b41f
8488060
de9dca5
a135136
f203cec
977df6f
c97d804
e8bcfcb
1033a23
632638e
6af080b
c01292c
c6d868e
527553b
c98601d
4301a35
2c4cb43
6fd31bd
c9be433
8a952e0
3a9a9fc
389316e
96fb26c
9795527
f94e5b3
9c5d0ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,9 @@ | |
package clusterapply | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"k8s.io/apimachinery/pkg/types" | ||
"time" | ||
|
||
ctldiff "github.com/k14s/kapp/pkg/kapp/diff" | ||
|
@@ -26,7 +28,9 @@ const ( | |
) | ||
|
||
type AddOrUpdateChangeOpts struct { | ||
DefaultUpdateStrategy string | ||
DefaultUpdateStrategy string | ||
ServerSideApply bool | ||
ServerSideForceConflict bool | ||
} | ||
|
||
type AddOrUpdateChange struct { | ||
|
@@ -131,7 +135,7 @@ func (c AddOrUpdateChange) tryToResolveUpdateConflict( | |
changeSet := c.changeSetFactory.New([]ctlres.Resource{latestExistingRes}, | ||
[]ctlres.Resource{c.change.AppliedResource()}) | ||
|
||
recalcChanges, err := changeSet.Calculate() | ||
recalcChanges, err := changeSet.Calculate(context.TODO()) | ||
if err != nil { | ||
return err | ||
} | ||
|
@@ -172,7 +176,7 @@ func (c AddOrUpdateChange) tryToUpdateAfterCreateConflict() error { | |
changeSet := c.changeSetFactory.New([]ctlres.Resource{latestExistingRes}, | ||
[]ctlres.Resource{c.change.AppliedResource()}) | ||
|
||
recalcChanges, err := changeSet.Calculate() | ||
recalcChanges, err := changeSet.Calculate(context.TODO()) | ||
if err != nil { | ||
return err | ||
} | ||
|
@@ -295,7 +299,17 @@ type UpdatePlainStrategy struct { | |
func (c UpdatePlainStrategy) Op() ClusterChangeApplyStrategyOp { return updateStrategyPlainAnnValue } | ||
|
||
func (c UpdatePlainStrategy) Apply() error { | ||
updatedRes, err := c.aou.identifiedResources.Update(c.newRes) | ||
var updatedRes ctlres.Resource | ||
var err error | ||
|
||
if c.aou.opts.ServerSideApply { | ||
updatedRes, err = ctlres.WithIdentityAnnotation(c.newRes, func(r ctlres.Resource) (ctlres.Resource, error) { | ||
resBytes, _ := r.AsYAMLBytes() | ||
redbaron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return c.aou.identifiedResources.Patch(r, types.ApplyPatchType, resBytes) | ||
}) | ||
} else { | ||
updatedRes, err = c.aou.identifiedResources.Update(c.newRes) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we be tryToResolveUpdateConflict doing when SSA is on? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right. |
||
} | ||
if err != nil { | ||
if errors.IsConflict(err) { | ||
return c.aou.tryToResolveUpdateConflict(err, func(err error) error { return err }) | ||
|
@@ -323,8 +337,20 @@ func (c UpdateOrFallbackOnReplaceStrategy) Apply() error { | |
return err | ||
} | ||
|
||
updatedRes, err := c.aou.identifiedResources.Update(c.newRes) | ||
var updatedRes ctlres.Resource | ||
var err error | ||
|
||
if c.aou.opts.ServerSideApply { | ||
updatedRes, err = ctlres.WithIdentityAnnotation(c.newRes, func(r ctlres.Resource) (ctlres.Resource, error) { | ||
resBytes, _ := r.AsYAMLBytes() | ||
return c.aou.identifiedResources.Patch(r, types.ApplyPatchType, resBytes) | ||
}) | ||
} else { | ||
updatedRes, err = c.aou.identifiedResources.Update(c.newRes) | ||
} | ||
|
||
if err != nil { | ||
//TODO: find out if SSA conflicts worth retrying | ||
if errors.IsConflict(err) { | ||
return c.aou.tryToResolveUpdateConflict(err, replaceIfIsInvalidErrFunc) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
package app | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"sort" | ||
"strings" | ||
|
@@ -51,7 +52,12 @@ func NewDeployCmd(o *DeployOptions, flagsFactory cmdcore.FlagsFactory) *cobra.Co | |
Use: "deploy", | ||
Aliases: []string{"d", "dep"}, | ||
Short: "Deploy app", | ||
RunE: func(_ *cobra.Command, _ []string) error { return o.Run() }, | ||
RunE: func(_ *cobra.Command, _ []string) error { | ||
if o.ApplyFlags.ServerSideApply { | ||
o.DiffFlags.ChangeSetOpts.Mode = ctldiff.ServerSideApplyChangeSetMode | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not a fan of doing it in RunE, lets move this into Run()... (also going to see if there is a better place for this) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I plan to add flag validation step and return error, rather than silently overwriting values provided by user, so it should be a RunE I believe because it can return error. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we could move these validations to the RunE: func(_ *cobra.Command, _ []string) error { return o.Run() } There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to PreRunE |
||
return o.Run() | ||
}, | ||
Annotations: map[string]string{ | ||
cmdcore.AppHelpGroup.Key: cmdcore.AppHelpGroup.Value, | ||
}, | ||
|
@@ -72,9 +78,10 @@ func NewDeployCmd(o *DeployOptions, flagsFactory cmdcore.FlagsFactory) *cobra.Co | |
|
||
o.AppFlags.Set(cmd, flagsFactory) | ||
o.FileFlags.Set(cmd) | ||
o.DiffFlags.SetWithPrefix("diff", cmd) | ||
o.ResourceFilterFlags.Set(cmd) | ||
o.ApplyFlags.SetWithDefaults("", ApplyFlagsDeployDefaults, cmd) | ||
o.DiffFlags.SetWithPrefix("diff", cmd) | ||
|
||
o.DeployFlags.Set(cmd) | ||
o.ResourceTypesFlags.Set(cmd) | ||
o.LabelFlags.Set(cmd) | ||
|
@@ -83,9 +90,11 @@ func NewDeployCmd(o *DeployOptions, flagsFactory cmdcore.FlagsFactory) *cobra.Co | |
} | ||
|
||
func (o *DeployOptions) Run() error { | ||
ctx := context.Background() | ||
|
||
failingAPIServicesPolicy := o.ResourceTypesFlags.FailingAPIServicePolicy() | ||
|
||
app, supportObjs, err := Factory(o.depsFactory, o.AppFlags, o.ResourceTypesFlags, o.logger) | ||
app, supportObjs, err := Factory(o.depsFactory, o.AppFlags, o.ResourceTypesFlags, o.logger, &o.ApplyFlags.FieldManagerName) | ||
if err != nil { | ||
return err | ||
} | ||
|
@@ -140,7 +149,7 @@ func (o *DeployOptions) Run() error { | |
} | ||
|
||
clusterChangeSet, clusterChangesGraph, hasNoChanges, changeSummary, err := | ||
o.calculateAndPresentChanges(existingResources, newResources, conf, supportObjs) | ||
o.calculateAndPresentChanges(ctx, existingResources, newResources, conf, supportObjs) | ||
if err != nil { | ||
if o.DiffFlags.UI && clusterChangesGraph != nil { | ||
return o.presentDiffUI(clusterChangesGraph) | ||
|
@@ -315,19 +324,19 @@ func (o *DeployOptions) existingResources(newResources []ctlres.Resource, | |
return resourceFilter.Apply(existingResources), o.existingPodResources(existingResources), nil | ||
} | ||
|
||
func (o *DeployOptions) calculateAndPresentChanges(existingResources, | ||
func (o *DeployOptions) calculateAndPresentChanges(ctx context.Context, existingResources, | ||
newResources []ctlres.Resource, conf ctlconf.Conf, supportObjs FactorySupportObjs) ( | ||
ctlcap.ClusterChangeSet, *ctldgraph.ChangeGraph, bool, string, error) { | ||
|
||
var clusterChangeSet ctlcap.ClusterChangeSet | ||
|
||
{ // Figure out changes for X existing resources -> X new resources | ||
changeFactory := ctldiff.NewChangeFactory(conf.RebaseMods(), conf.DiffAgainstLastAppliedFieldExclusionMods()) | ||
changeFactory := ctldiff.NewChangeFactory(conf.RebaseMods(), conf.DiffAgainstLastAppliedFieldExclusionMods(), supportObjs.Resources) | ||
changeSetFactory := ctldiff.NewChangeSetFactory(o.DiffFlags.ChangeSetOpts, changeFactory) | ||
|
||
changes, err := ctldiff.NewChangeSetWithVersionedRs( | ||
existingResources, newResources, conf.TemplateRules(), | ||
o.DiffFlags.ChangeSetOpts, changeFactory).Calculate() | ||
o.DiffFlags.ChangeSetOpts, changeFactory).Calculate(ctx) | ||
if err != nil { | ||
return clusterChangeSet, nil, false, "", err | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems that we have two types of Patch operations: one where we are working with a resource, and one where we are working with some "minimal/raw" patch. instead of exposing WithIdentityAnnotation outside of IdentifiedResources resources class, lets keep existing Patch method to work with Resource class (instead of []byte) and change it so that it can add annotation on the fly. lets also add PatchRaw which does take []byte. this should result in keeping WithIdentityAnnotation as private.