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
Merged
2 changes: 1 addition & 1 deletion deploy/skaffold/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ RUN apt-get update && \
apt-get install --no-install-recommends --no-install-suggests -y \
python-dev

ENV KUBECTL_VERSION v1.10.0
ENV KUBECTL_VERSION v1.12.0
RUN curl -Lo /usr/local/bin/kubectl https://storage.googleapis.com/kubernetes-release/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl && \
chmod +x /usr/local/bin/kubectl

Expand Down
2 changes: 2 additions & 0 deletions docs/concepts.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ tools for deployment, for example `kubectl` or `helm`.
Each deployment type has parameters that allow you to
define how you want your app to be installed and updated.

_Note: kubectl version 1.12.0 or greater is recommended for use with skaffold._
Copy link
Contributor

Choose a reason for hiding this comment

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

might be worth documenting the known issues for earlier kubectl versions for users

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for sure, would this be the right spot for that?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, sorry missed your reply on this. FWIW this is a fine place to do it :)


=== Artifact
Artifacts are the outputs of the build phase.
Artifacts are created by running a set of steps on some
Expand Down
3 changes: 3 additions & 0 deletions integration/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 integration/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 integration/examples/test-dev-job/skaffold.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
apiVersion: skaffold/v1alpha4
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 @@ -38,8 +38,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 @@ -206,6 +208,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("redeploy failed: %v", err)
}
},
},
}

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
3 changes: 3 additions & 0 deletions pkg/skaffold/deploy/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ func (k *KubectlDeployer) Labels() map[string]string {
// runs `kubectl apply` on those manifests
func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) ([]Artifact, error) {
color.Default.Fprintln(out, "kubectl client version:", k.kubectl.Version())
if err := k.kubectl.CheckVersion(); err != nil {
color.Default.Fprintln(out, err)
}

manifests, err := k.readManifests(ctx)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion pkg/skaffold/deploy/kubectl/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ func (c *CLI) Apply(ctx context.Context, out io.Writer, manifests ManifestList)
return nil, nil
}

if err := c.Run(ctx, updated.Reader(), out, "apply", c.Flags.Apply, "-f", "-"); err != nil {
// Add --force flag to delete and redeploy image if changes can't be applied
if err := c.Run(ctx, updated.Reader(), out, "apply", c.Flags.Apply, "--force", "-f", "-"); err != nil {
return nil, errors.Wrap(err, "kubectl apply")
}

Expand Down
14 changes: 14 additions & 0 deletions pkg/skaffold/deploy/kubectl/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import (
"context"
"encoding/json"
"os/exec"
"strconv"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

Expand All @@ -46,6 +48,18 @@ func (v ClientVersion) String() string {
return v.Major + "." + v.Minor
}

// CheckVersion warns the user if their kubectl version is < 1.12.0
func (c *CLI) CheckVersion() error {
m, err := strconv.Atoi(c.Version().Minor)
if err != nil {
return errors.Wrap(err, "couldn't get kubectl minor version")
}
if m < 12 {
return errors.New("kubectl version 1.12.0 or greater is recommended for use with skaffold")
}
return nil
}

// Version returns the client version of kubectl.
func (c *CLI) Version() ClientVersion {
c.versionOnce.Do(func() {
Expand Down
8 changes: 4 additions & 4 deletions pkg/skaffold/deploy/kubectl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestKubectlDeploy(t *testing.T) {
cfg: &latest.KubectlDeploy{
Manifests: []string{"deployment.yaml"},
},
command: testutil.NewFakeCmd("kubectl --context kubecontext --namespace testNamespace apply -f -", nil),
command: testutil.NewFakeCmd("kubectl --context kubecontext --namespace testNamespace apply --force -f -", nil),
builds: []build.Artifact{
{
ImageName: "leeroy-web",
Expand All @@ -102,7 +102,7 @@ func TestKubectlDeploy(t *testing.T) {
cfg: &latest.KubectlDeploy{
Manifests: []string{"deployment.yaml"},
},
command: testutil.NewFakeCmd("kubectl --context kubecontext --namespace testNamespace apply -f -", fmt.Errorf("")),
command: testutil.NewFakeCmd("kubectl --context kubecontext --namespace testNamespace apply --force -f -", fmt.Errorf("")),
builds: []build.Artifact{
{
ImageName: "leeroy-web",
Expand All @@ -121,7 +121,7 @@ func TestKubectlDeploy(t *testing.T) {
Delete: []string{"ignored"},
},
},
command: testutil.NewFakeCmd("kubectl --context kubecontext --namespace testNamespace -v=0 apply -f -", fmt.Errorf("")),
command: testutil.NewFakeCmd("kubectl --context kubecontext --namespace testNamespace -v=0 apply --force -f -", fmt.Errorf("")),
builds: []build.Artifact{
{
ImageName: "leeroy-web",
Expand Down Expand Up @@ -209,7 +209,7 @@ func TestKubectlCleanup(t *testing.T) {

func TestKubectlRedeploy(t *testing.T) {
defer func(c util.Command) { util.DefaultExecCommand = c }(util.DefaultExecCommand)
util.DefaultExecCommand = testutil.NewFakeCmd("kubectl --context kubecontext --namespace testNamespace apply -f -", nil)
util.DefaultExecCommand = testutil.NewFakeCmd("kubectl --context kubecontext --namespace testNamespace apply --force -f -", nil)

tmpDir, cleanup := testutil.NewTempDir(t)
defer cleanup()
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
})
}