Skip to content

Commit

Permalink
Add force-sync-by-replace annotation option (#5175)
Browse files Browse the repository at this point in the history
* Add test for force-sync-by-replace

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Add ForceReplaceManifest methods

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Add force-sync-by-replace handling

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Fix the godoc comment

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

---------

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>
  • Loading branch information
Warashi authored and pipecd-bot committed Sep 4, 2024
1 parent 7db1a06 commit f621fb6
Show file tree
Hide file tree
Showing 6 changed files with 232 additions and 14 deletions.
49 changes: 35 additions & 14 deletions pkg/app/piped/executor/kubernetes/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,27 +240,48 @@ func applyManifests(ctx context.Context, ag applierGetter, manifests []provider.
return err
}

annotation := m.GetAnnotations()[provider.LabelSyncReplace]
if annotation != provider.UseReplaceEnabled {
if err := applier.ApplyManifest(ctx, m); err != nil {
lp.Errorf("Failed to apply manifest: %s (%w)", m.Key.ReadableString(), err)
// The force annotation has higher priority, so we need to check the annotation in the following order:
// 1. force-sync-by-replace
// 2. sync-by-replace
// 3. others
if annotation := m.GetAnnotations()[provider.LabelForceSyncReplace]; annotation == provider.UseReplaceEnabled {
// Always try to replace first and create if it fails due to resource not found error.
// This is because we cannot know whether resource already exists before executing command.
err = applier.ForceReplaceManifest(ctx, m)
if errors.Is(err, provider.ErrNotFound) {
lp.Infof("Specified resource does not exist, so create the resource: %s (%w)", m.Key.ReadableString(), err)
err = applier.CreateManifest(ctx, m)
}
if err != nil {
lp.Errorf("Failed to forcefully replace or create manifest: %s (%w)", m.Key.ReadableString(), err)
return err
}
lp.Successf("- applied manifest: %s", m.Key.ReadableString())
lp.Successf("- forcefully replaced or created manifest: %s", m.Key.ReadableString())
continue
}
// Always try to replace first and create if it fails due to resource not found error.
// This is because we cannot know whether resource already exists before executing command.
err = applier.ReplaceManifest(ctx, m)
if errors.Is(err, provider.ErrNotFound) {
lp.Infof("Specified resource does not exist, so create the resource: %s (%w)", m.Key.ReadableString(), err)
err = applier.CreateManifest(ctx, m)

if annotation := m.GetAnnotations()[provider.LabelSyncReplace]; annotation == provider.UseReplaceEnabled {
// Always try to replace first and create if it fails due to resource not found error.
// This is because we cannot know whether resource already exists before executing command.
err = applier.ReplaceManifest(ctx, m)
if errors.Is(err, provider.ErrNotFound) {
lp.Infof("Specified resource does not exist, so create the resource: %s (%w)", m.Key.ReadableString(), err)
err = applier.CreateManifest(ctx, m)
}
if err != nil {
lp.Errorf("Failed to replace or create manifest: %s (%w)", m.Key.ReadableString(), err)
return err
}
lp.Successf("- replaced or created manifest: %s", m.Key.ReadableString())
continue
}
if err != nil {
lp.Errorf("Failed to replace or create manifest: %s (%w)", m.Key.ReadableString(), err)

if err := applier.ApplyManifest(ctx, m); err != nil {
lp.Errorf("Failed to apply manifest: %s (%w)", m.Key.ReadableString(), err)
return err
}
lp.Successf("- replaced or created manifest: %s", m.Key.ReadableString())
lp.Successf("- applied manifest: %s", m.Key.ReadableString())
continue

}
lp.Successf("Successfully applied %d manifests", len(manifests))
Expand Down
106 changes: 106 additions & 0 deletions pkg/app/piped/executor/kubernetes/kubernetes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,32 @@ spec:
metadata:
labels:
app: simple
`,
namespace: "",
wantErr: true,
},
{
name: "unable to force replace manifest",
applier: func() provider.Applier {
p := kubernetestest.NewMockApplier(ctrl)
p.EXPECT().ForceReplaceManifest(gomock.Any(), gomock.Any()).Return(errors.New("unexpected error"))
return p
}(),
manifest: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: simple
annotations:
pipecd.dev/force-sync-by-replace: "enabled"
spec:
selector:
matchLabels:
app: simple
template:
metadata:
labels:
app: simple
`,
namespace: "",
wantErr: true,
Expand Down Expand Up @@ -382,6 +408,33 @@ spec:
metadata:
labels:
app: simple
`,
namespace: "",
wantErr: true,
},
{
name: "unable to create manifest",
applier: func() provider.Applier {
p := kubernetestest.NewMockApplier(ctrl)
p.EXPECT().ForceReplaceManifest(gomock.Any(), gomock.Any()).Return(provider.ErrNotFound)
p.EXPECT().CreateManifest(gomock.Any(), gomock.Any()).Return(errors.New("unexpected error"))
return p
}(),
manifest: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: simple
annotations:
pipecd.dev/force-sync-by-replace: "enabled"
spec:
selector:
matchLabels:
app: simple
template:
metadata:
labels:
app: simple
`,
namespace: "",
wantErr: true,
Expand Down Expand Up @@ -432,6 +485,32 @@ spec:
metadata:
labels:
app: simple
`,
namespace: "",
wantErr: false,
},
{
name: "successfully force replace manifest",
applier: func() provider.Applier {
p := kubernetestest.NewMockApplier(ctrl)
p.EXPECT().ForceReplaceManifest(gomock.Any(), gomock.Any()).Return(nil)
return p
}(),
manifest: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: simple
annotations:
pipecd.dev/force-sync-by-replace: "enabled"
spec:
selector:
matchLabels:
app: simple
template:
metadata:
labels:
app: simple
`,
namespace: "",
wantErr: false,
Expand Down Expand Up @@ -459,6 +538,33 @@ spec:
metadata:
labels:
app: simple
`,
namespace: "",
wantErr: false,
},
{
name: "successfully force create manifest",
applier: func() provider.Applier {
p := kubernetestest.NewMockApplier(ctrl)
p.EXPECT().ForceReplaceManifest(gomock.Any(), gomock.Any()).Return(provider.ErrNotFound)
p.EXPECT().CreateManifest(gomock.Any(), gomock.Any()).Return(nil)
return p
}(),
manifest: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: simple
annotations:
pipecd.dev/force-sync-by-replace: "enabled"
spec:
selector:
matchLabels:
app: simple
template:
metadata:
labels:
app: simple
`,
namespace: "",
wantErr: false,
Expand Down
37 changes: 37 additions & 0 deletions pkg/app/piped/platformprovider/kubernetes/applier.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ type Applier interface {
CreateManifest(ctx context.Context, manifest Manifest) error
// ReplaceManifest does replacing resource from given manifest.
ReplaceManifest(ctx context.Context, manifest Manifest) error
// ForceReplaceManifest does force replacing resource from given manifest.
ForceReplaceManifest(ctx context.Context, manifest Manifest) error
// Delete deletes the given resource from Kubernetes cluster.
Delete(ctx context.Context, key ResourceKey) error
}
Expand Down Expand Up @@ -137,6 +139,32 @@ func (a *applier) ReplaceManifest(ctx context.Context, manifest Manifest) error
return err
}

// ForceReplaceManifest uses kubectl to forcefully replace the given manifests.
func (a *applier) ForceReplaceManifest(ctx context.Context, manifest Manifest) error {
a.initOnce.Do(func() {
a.kubectl, a.initErr = a.findKubectl(ctx, a.getToolVersionToRun())
})
if a.initErr != nil {
return a.initErr
}

Check warning on line 149 in pkg/app/piped/platformprovider/kubernetes/applier.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/kubernetes/applier.go#L143-L149

Added lines #L143 - L149 were not covered by tests

err := a.kubectl.ForceReplace(
ctx,
a.platformProvider.KubeConfigPath,
a.getNamespaceToRun(manifest.Key),
manifest,
)
if err == nil {
return nil
}

Check warning on line 159 in pkg/app/piped/platformprovider/kubernetes/applier.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/kubernetes/applier.go#L151-L159

Added lines #L151 - L159 were not covered by tests

if errors.Is(err, errorReplaceNotFound) {
return ErrNotFound
}

Check warning on line 163 in pkg/app/piped/platformprovider/kubernetes/applier.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/kubernetes/applier.go#L161-L163

Added lines #L161 - L163 were not covered by tests

return err

Check warning on line 165 in pkg/app/piped/platformprovider/kubernetes/applier.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/kubernetes/applier.go#L165

Added line #L165 was not covered by tests
}

// Delete deletes the given resource from Kubernetes cluster.
// If the resource key is different, this returns ErrNotFound.
func (a *applier) Delete(ctx context.Context, k ResourceKey) (err error) {
Expand Down Expand Up @@ -237,6 +265,15 @@ func (a *multiApplier) ReplaceManifest(ctx context.Context, manifest Manifest) e
return nil
}

func (a *multiApplier) ForceReplaceManifest(ctx context.Context, manifest Manifest) error {
for _, a := range a.appliers {
if err := a.ForceReplaceManifest(ctx, manifest); err != nil {
return err
}

Check warning on line 272 in pkg/app/piped/platformprovider/kubernetes/applier.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/kubernetes/applier.go#L268-L272

Added lines #L268 - L272 were not covered by tests
}
return nil

Check warning on line 274 in pkg/app/piped/platformprovider/kubernetes/applier.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/kubernetes/applier.go#L274

Added line #L274 was not covered by tests
}

func (a *multiApplier) Delete(ctx context.Context, key ResourceKey) error {
for _, a := range a.appliers {
if err := a.Delete(ctx, key); err != nil {
Expand Down
39 changes: 39 additions & 0 deletions pkg/app/piped/platformprovider/kubernetes/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,45 @@ func (c *Kubectl) Replace(ctx context.Context, kubeconfig, namespace string, man
return fmt.Errorf("failed to replace: %s (%w)", string(out), err)
}

func (c *Kubectl) ForceReplace(ctx context.Context, kubeconfig, namespace string, manifest Manifest) (err error) {
defer func() {
kubernetesmetrics.IncKubectlCallsCounter(
c.version,
kubernetesmetrics.LabelReplaceCommand,
err == nil,
)
}()

Check warning on line 169 in pkg/app/piped/platformprovider/kubernetes/kubectl.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/kubernetes/kubectl.go#L162-L169

Added lines #L162 - L169 were not covered by tests

data, err := manifest.YamlBytes()
if err != nil {
return err
}

Check warning on line 174 in pkg/app/piped/platformprovider/kubernetes/kubectl.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/kubernetes/kubectl.go#L171-L174

Added lines #L171 - L174 were not covered by tests

args := make([]string, 0, 7)
if kubeconfig != "" {
args = append(args, "--kubeconfig", kubeconfig)
}
if namespace != "" {
args = append(args, "--namespace", namespace)
}
args = append(args, "replace", "--force", "-f", "-")

cmd := exec.CommandContext(ctx, c.execPath, args...)
r := bytes.NewReader(data)
cmd.Stdin = r

out, err := cmd.CombinedOutput()
if err == nil {
return nil
}

Check warning on line 192 in pkg/app/piped/platformprovider/kubernetes/kubectl.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/kubernetes/kubectl.go#L176-L192

Added lines #L176 - L192 were not covered by tests

if strings.Contains(string(out), errorNotFoundLiteral) {
return errorReplaceNotFound
}

Check warning on line 196 in pkg/app/piped/platformprovider/kubernetes/kubectl.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/kubernetes/kubectl.go#L194-L196

Added lines #L194 - L196 were not covered by tests

return fmt.Errorf("failed to replace: %s (%w)", string(out), err)

Check warning on line 198 in pkg/app/piped/platformprovider/kubernetes/kubectl.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/piped/platformprovider/kubernetes/kubectl.go#L198

Added line #L198 was not covered by tests
}

func (c *Kubectl) Delete(ctx context.Context, kubeconfig, namespace string, r ResourceKey) (err error) {
defer func() {
kubernetesmetrics.IncKubectlCallsCounter(
Expand Down
1 change: 1 addition & 0 deletions pkg/app/piped/platformprovider/kubernetes/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const (
LabelOriginalAPIVersion = "pipecd.dev/original-api-version" // The api version defined in git configuration. e.g. apps/v1
LabelIgnoreDriftDirection = "pipecd.dev/ignore-drift-detection" // Whether the drift detection should ignore this resource.
LabelSyncReplace = "pipecd.dev/sync-by-replace" // Use replace instead of apply.
LabelForceSyncReplace = "pipecd.dev/force-sync-by-replace" // Use replace --force instead of apply.
LabelServerSideApply = "pipecd.dev/server-side-apply" // Use server side apply instead of client side apply.
AnnotationConfigHash = "pipecd.dev/config-hash" // The hash value of all mouting config resources.
AnnotationOrder = "pipecd.dev/order" // The order number of resource used to sort them before using.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit f621fb6

Please sign in to comment.