Skip to content
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

Delete and redeploy object upon error 'field is immutable' #940

Merged
merged 10 commits into from
Oct 10, 2018
3 changes: 3 additions & 0 deletions examples/test-dev-job/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
FROM golang:1.10.1-alpine3.7 as builder
COPY foo /foo
CMD sleep 600
11 changes: 11 additions & 0 deletions examples/test-dev-job/k8s-job.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
apiVersion: batch/v1
kind: Job
metadata:
name: test-dev-job
spec:
template:
spec:
containers:
- name: test-dev-job
image: gcr.io/k8s-skaffold/test-dev-job
restartPolicy: OnFailure
14 changes: 14 additions & 0 deletions examples/test-dev-job/skaffold.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
apiVersion: skaffold/v1alpha2
kind: Config
build:
artifacts:
- imageName: gcr.io/k8s-skaffold/test-dev-job
deploy:
kubectl:
manifests:
- k8s-*
profiles:
- name: gcb
build:
googleCloudBuild:
projectId: k8s-skaffold
94 changes: 94 additions & 0 deletions integration/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ import (
"github.com/sirupsen/logrus"
"gopkg.in/yaml.v2"
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
"k8s.io/api/core/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
)

Expand Down Expand Up @@ -204,6 +206,98 @@ func TestRun(t *testing.T) {
}
}

func TestDev(t *testing.T) {
type testDevCase struct {
description string
dir string
args []string
setup func(t *testing.T) func(t *testing.T)
jobs []string
jobValidation func(t *testing.T, ns *v1.Namespace, j *batchv1.Job)
}

testCases := []testDevCase{
{
description: "delete and redeploy job",
dir: "../examples/test-dev-job",
args: []string{"dev"},
setup: func(t *testing.T) func(t *testing.T) {
// create foo
cmd := exec.Command("touch", "../examples/test-dev-job/foo")
if output, err := util.RunCmdOut(cmd); err != nil {
t.Fatalf("creating foo: %s %v", output, err)
}
return func(t *testing.T) {
// delete foo
cmd := exec.Command("rm", "../examples/test-dev-job/foo")
if output, err := util.RunCmdOut(cmd); err != nil {
t.Fatalf("creating foo: %s %v", output, err)
}
}
},
jobs: []string{
"test-dev-job",
},
jobValidation: func(t *testing.T, ns *v1.Namespace, j *batchv1.Job) {
originalUID := j.GetUID()
// Make a change to foo so that dev is forced to delete the job and redeploy
cmd := exec.Command("sh", "-c", "echo bar > ../examples/test-dev-job/foo")
if output, err := util.RunCmdOut(cmd); err != nil {
t.Fatalf("creating bar: %s %v", output, err)
}
// Make sure the UID of the old Job and the UID of the new Job is different
err := wait.PollImmediate(time.Millisecond*500, 10*time.Minute, func() (bool, error) {
newJob, err := client.BatchV1().Jobs(ns.Name).Get(j.Name, meta_v1.GetOptions{})
if err != nil {
return false, nil
}
return originalUID != newJob.GetUID(), nil
})
if err != nil {
t.Fatalf("original UID and new UID are the same, redeploy failed")
}
},
},
}

for _, testCase := range testCases {
t.Run(testCase.description, func(t *testing.T) {
ns, deleteNs := setupNamespace(t)
defer deleteNs()

cleanupTC := testCase.setup(t)
defer cleanupTC(t)

args := []string{}
args = append(args, testCase.args...)
args = append(args, "--namespace", ns.Name)

cmd := exec.Command("skaffold", args...)
cmd.Dir = testCase.dir
go func() {
if output, err := util.RunCmdOut(cmd); err != nil {
logrus.Warnf("skaffold: %s %v", output, err)
}
}()

for _, j := range testCase.jobs {
if err := kubernetesutil.WaitForJobToStabilize(client, ns.Name, j, 10*time.Minute); err != nil {
t.Fatalf("Timed out waiting for job to stabilize")
}
if testCase.jobValidation != nil {
job, err := client.BatchV1().Jobs(ns.Name).Get(j, meta_v1.GetOptions{})
if err != nil {
t.Fatalf("Could not find job: %s %s", ns.Name, j)
}
testCase.jobValidation(t, ns, job)
}
}

// No cleanup, since exiting skaffold dev should clean up automatically
})
}
}

func setupNamespace(t *testing.T) (*v1.Namespace, func()) {
ns, err := client.CoreV1().Namespaces().Create(&v1.Namespace{
ObjectMeta: meta_v1.ObjectMeta{
Expand Down
29 changes: 25 additions & 4 deletions pkg/skaffold/deploy/kubectl/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@ limitations under the License.
package kubectl

import (
"bufio"
"bytes"
"context"
"io"
"os/exec"
"strings"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
Expand Down Expand Up @@ -55,11 +58,29 @@ func (c *CLI) Apply(ctx context.Context, out io.Writer, manifests ManifestList)
if len(updated) == 0 {
return nil, nil
}

if err := c.Run(ctx, manifests.Reader(), out, "apply", c.Flags.Apply, "-f", "-"); err != nil {
return nil, errors.Wrap(err, "kubectl apply")
for _, mfst := range manifests {
buf := bytes.NewBuffer([]byte{})
writer := bufio.NewWriter(buf)
ml := ManifestList{mfst}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to convert this back to a manifest list, mfst is just a []byte at this point

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I had to convert it because the Delete function takes in a ManifestList

func (c *CLI) Delete(ctx context.Context, out io.Writer, manifests ManifestList) error

if err := c.Run(ctx, ml.Reader(), writer, "apply", c.Flags.Apply, "-f", "-"); err != nil {
if !strings.Contains(buf.String(), "field is immutable") {
return nil, err
}
// If the output contains the string 'field is immutable', we want to delete the object and recreate it
// See Issue #891 for more information
if err := c.Delete(ctx, out, ml); err != nil {
return nil, errors.Wrap(err, "deleting manifest")
}
if err := c.Run(ctx, ml.Reader(), out, "apply", c.Flags.Apply, "-f", "-"); err != nil {
return nil, errors.Wrap(err, "kubectl apply after deletion")
}
} else {
// Write output to out
if _, err := out.Write(buf.Bytes()); err != nil {
return nil, errors.Wrap(err, "writing to out")
}
}
}

return updated, nil
}

Expand Down
11 changes: 11 additions & 0 deletions pkg/skaffold/kubernetes/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,14 @@ func WaitForDeploymentToStabilize(c kubernetes.Interface, ns, name string, timeo
})
return err
}

// WaitForJobToStabilize waits till the Job has at least one active pod
func WaitForJobToStabilize(c kubernetes.Interface, ns, name string, timeout time.Duration) error {
return wait.PollImmediate(time.Millisecond*500, timeout, func() (bool, error) {
job, err := c.BatchV1().Jobs(ns).Get(name, meta_v1.GetOptions{})
if err != nil {
return false, nil
}
return job.Status.Active > 0, nil
})
}