code review (error checks, collapse forceConflicts, UpdateApplyFunc)

Signed-off-by: George Jenkins <gvjenkins@gmail.com>
pull/31030/head
George Jenkins 3 weeks ago
parent 99dc23f00b
commit b2dc411f9d

@ -75,7 +75,7 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent,
// Create hook resources
if _, err := cfg.KubeClient.Create(
resources,
kube.ClientCreateOptionServerSideApply(false)); err != nil {
kube.ClientCreateOptionServerSideApply(false, false)); err != nil {
h.LastRun.CompletedAt = helmtime.Now()
h.LastRun.Phase = release.HookPhaseFailed
return fmt.Errorf("warning: Hook %s %s failed: %w", hook, h.Path, err)

@ -175,7 +175,7 @@ func (i *Install) installCRDs(crds []chart.CRD) error {
// Send them to Kube
if _, err := i.cfg.KubeClient.Create(
res,
kube.ClientCreateOptionServerSideApply(false)); err != nil {
kube.ClientCreateOptionServerSideApply(false, false)); err != nil {
// If the error is CRD already exists, continue.
if apierrors.IsAlreadyExists(err) {
crdName := res[0].Name
@ -403,7 +403,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
}
if _, err := i.cfg.KubeClient.Create(
resourceList,
kube.ClientCreateOptionServerSideApply(false)); err != nil && !apierrors.IsAlreadyExists(err) {
kube.ClientCreateOptionServerSideApply(false, false)); err != nil && !apierrors.IsAlreadyExists(err) {
return nil, err
}
}
@ -474,13 +474,13 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource
if len(toBeAdopted) == 0 && len(resources) > 0 {
_, err = i.cfg.KubeClient.Create(
resources,
kube.ClientCreateOptionServerSideApply(false))
kube.ClientCreateOptionServerSideApply(false, false))
} else if len(resources) > 0 {
updateThreeWayMergeForUnstructured := i.TakeOwnership
_, err = i.cfg.KubeClient.Update(
toBeAdopted,
resources,
kube.ClientUpdateOptionServerSideApply(false),
kube.ClientUpdateOptionServerSideApply(false, false),
kube.ClientUpdateOptionThreeWayMergeForUnstructured(updateThreeWayMergeForUnstructured),
kube.ClientUpdateOptionForceReplace(i.ForceReplace))
}

@ -193,7 +193,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas
results, err := r.cfg.KubeClient.Update(
current,
target,
kube.ClientUpdateOptionServerSideApply(false),
kube.ClientUpdateOptionServerSideApply(false, false),
kube.ClientUpdateOptionForceReplace(r.ForceReplace))
if err != nil {

@ -429,7 +429,7 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele
results, err := u.cfg.KubeClient.Update(
current,
target,
kube.ClientUpdateOptionServerSideApply(false),
kube.ClientUpdateOptionServerSideApply(false, false),
kube.ClientUpdateOptionForceReplace(u.ForceReplace))
if err != nil {
u.cfg.recordRelease(originalRelease)

@ -214,26 +214,23 @@ type ClientCreateOption func(*clientCreateOptions) error
// ClientUpdateOptionServerSideApply enables performing object apply server-side
// see: https://kubernetes.io/docs/reference/using-api/server-side-apply/
func ClientCreateOptionServerSideApply(serverSideApply bool) ClientCreateOption {
return func(o *clientCreateOptions) error {
o.serverSideApply = serverSideApply
return nil
}
}
// ClientCreateOptionForceConflicts forces field conflicts to be resolved
//
// `forceConflicts` forces conflicts to be resolved (may be when serverSideApply enabled only)
// see: https://kubernetes.io/docs/reference/using-api/server-side-apply/#conflicts
// Only valid when ClientUpdateOptionServerSideApply enabled
func ClientCreateOptionForceConflicts(forceConflicts bool) ClientCreateOption {
func ClientCreateOptionServerSideApply(serverSideApply, forceConflicts bool) ClientCreateOption {
return func(o *clientCreateOptions) error {
if !serverSideApply && forceConflicts {
return fmt.Errorf("forceConflicts enabled when serverSideApply disabled")
}
o.serverSideApply = serverSideApply
o.forceConflicts = forceConflicts
return nil
}
}
// ClientCreateOptionDryRun performs non-mutating operations only
// ClientCreateOptionDryRun requests the server to perform non-mutating operations only
func ClientCreateOptionDryRun(dryRun bool) ClientCreateOption {
return func(o *clientCreateOptions) error {
o.dryRun = dryRun
@ -264,8 +261,12 @@ func (c *Client) Create(resources ResourceList, options ...ClientCreateOption) (
fieldValidationDirective: FieldValidationDirectiveStrict,
}
errs := make([]error, 0, len(options))
for _, o := range options {
o(&createOptions)
errs = append(errs, o(&createOptions))
}
if err := errors.Join(errs...); err != nil {
return nil, fmt.Errorf("invalid client create option(s): %w", err)
}
if createOptions.forceConflicts && !createOptions.serverSideApply {
@ -499,7 +500,7 @@ func (c *Client) BuildTable(reader io.Reader, validate bool) (ResourceList, erro
transformRequests)
}
func (c *Client) update(originals, targets ResourceList, updateApplyFunc func(original, target *resource.Info) error) (*Result, error) {
func (c *Client) update(originals, targets ResourceList, updateApplyFunc UpdateApplyFunc) (*Result, error) {
updateErrors := []error{}
res := &Result{}
@ -599,9 +600,17 @@ func ClientUpdateOptionThreeWayMergeForUnstructured(threeWayMergeForUnstructured
// ClientUpdateOptionServerSideApply enables performing object apply server-side (default)
// see: https://kubernetes.io/docs/reference/using-api/server-side-apply/
// Must not be enabled when ClientUpdateOptionThreeWayMerge is enabled
func ClientUpdateOptionServerSideApply(serverSideApply bool) ClientUpdateOption {
//
// `forceConflicts` forces conflicts to be resolved (may be enabled when serverSideApply enabled only)
// see: https://kubernetes.io/docs/reference/using-api/server-side-apply/#conflicts
func ClientUpdateOptionServerSideApply(serverSideApply, forceConflicts bool) ClientUpdateOption {
return func(o *clientUpdateOptions) error {
if !serverSideApply && forceConflicts {
return fmt.Errorf("forceConflicts enabled when serverSideApply disabled")
}
o.serverSideApply = serverSideApply
o.forceConflicts = forceConflicts
return nil
}
@ -617,20 +626,7 @@ func ClientUpdateOptionForceReplace(forceReplace bool) ClientUpdateOption {
}
}
// ClientUpdateOptionForceConflicts forces field conflicts to be resolved
// see: https://kubernetes.io/docs/reference/using-api/server-side-apply/#conflicts
// Must not be enabled when ClientUpdateOptionForceReplace is enabled
func ClientUpdateOptionForceConflicts(forceConflicts bool) ClientUpdateOption {
return func(o *clientUpdateOptions) error {
o.forceConflicts = forceConflicts
return nil
}
}
// ClientUpdateOptionForceConflicts forces field conflicts to be resolved
// see: https://kubernetes.io/docs/reference/using-api/server-side-apply/#conflicts
// Must not be enabled when ClientUpdateOptionForceReplace is enabled
// ClientUpdateOptionDryRun requests the server to perform non-mutating operations only
func ClientUpdateOptionDryRun(dryRun bool) ClientUpdateOption {
return func(o *clientUpdateOptions) error {
o.dryRun = dryRun
@ -652,6 +648,8 @@ func ClientUpdateOptionFieldValidationDirective(fieldValidationDirective FieldVa
}
}
type UpdateApplyFunc func(original, target *resource.Info) error
// Update takes the current list of objects and target list of objects and
// creates resources that don't already exist, updates resources that have been
// modified in the target configuration, and deletes resources from the current
@ -667,8 +665,12 @@ func (c *Client) Update(originals, targets ResourceList, options ...ClientUpdate
fieldValidationDirective: FieldValidationDirectiveStrict,
}
errs := make([]error, 0, len(options))
for _, o := range options {
o(&updateOptions)
errs = append(errs, o(&updateOptions))
}
if err := errors.Join(errs...); err != nil {
return nil, fmt.Errorf("invalid client update option(s): %w", err)
}
if updateOptions.threeWayMergeForUnstructured && updateOptions.serverSideApply {
@ -683,7 +685,7 @@ func (c *Client) Update(originals, targets ResourceList, options ...ClientUpdate
return nil, fmt.Errorf("invalid operation: cannot use server-side apply and force replace together")
}
makeUpdateApplyFunc := func() func(original, target *resource.Info) error {
makeUpdateApplyFunc := func() UpdateApplyFunc {
if updateOptions.forceReplace {
slog.Debug(
"using resource replace update strategy",

@ -292,7 +292,7 @@ func TestCreate(t *testing.T) {
result, err := c.Create(
list,
ClientCreateOptionServerSideApply(tc.ServerSideApply))
ClientCreateOptionServerSideApply(tc.ServerSideApply, false))
if tc.ExpectedErrorContains != "" {
require.ErrorContains(t, err, tc.ExpectedErrorContains)
} else {
@ -467,7 +467,7 @@ func TestUpdate(t *testing.T) {
second,
ClientUpdateOptionThreeWayMergeForUnstructured(tc.ThreeWayMergeForUnstructured),
ClientUpdateOptionForceReplace(false),
ClientUpdateOptionServerSideApply(tc.ServerSideApply))
ClientUpdateOptionServerSideApply(tc.ServerSideApply, false))
require.NoError(t, err)
assert.Len(t, result.Created, 1, "expected 1 resource created, got %d", len(result.Created))
@ -684,7 +684,7 @@ func TestWait(t *testing.T) {
result, err := c.Create(
resources,
ClientCreateOptionServerSideApply(false))
ClientCreateOptionServerSideApply(false, false))
if err != nil {
t.Fatal(err)
@ -744,7 +744,7 @@ func TestWaitJob(t *testing.T) {
}
result, err := c.Create(
resources,
ClientCreateOptionServerSideApply(false))
ClientCreateOptionServerSideApply(false, false))
if err != nil {
t.Fatal(err)
@ -806,7 +806,7 @@ func TestWaitDelete(t *testing.T) {
}
result, err := c.Create(
resources,
ClientCreateOptionServerSideApply(false))
ClientCreateOptionServerSideApply(false, false))
if err != nil {
t.Fatal(err)
}
@ -1225,7 +1225,6 @@ func TestCreatePatchCustomResourceSpec(t *testing.T) {
t.Run(testCase.name, testCase.run)
}
<<<<<<< HEAD
type errorFactory struct {
*cmdtesting.TestFactory
err error
@ -1326,8 +1325,8 @@ func TestIsReachable(t *testing.T) {
}
})
}
||||||| parent of 36a476ff4 (Kube client support server-side apply)
=======
}
func TestIsIncompatibleServerError(t *testing.T) {
testCases := map[string]struct {
Err error
@ -1749,5 +1748,4 @@ func TestDetermineFieldValidationDirective(t *testing.T) {
assert.Equal(t, FieldValidationDirectiveIgnore, determineFieldValidationDirective(false))
assert.Equal(t, FieldValidationDirectiveStrict, determineFieldValidationDirective(true))
>>>>>>> 36a476ff4 (Kube client support server-side apply)
}

Loading…
Cancel
Save