Skip to content

Skaffold deploy hydrated manifests #4316

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

Merged
merged 11 commits into from
Jun 22, 2020
Merged

Skaffold deploy hydrated manifests #4316

merged 11 commits into from
Jun 22, 2020

Conversation

ChrisGe4
Copy link
Contributor

Description
Proposal: go/skaffold-deploy-hydrated-manifests

Currently skaffold deploy renders the Kubernetes manifests first then deploys the final manifests . This PR modifies modifies skaffold deploy to solely deploy the hydrated Kubernetes manifests.

In this PR, a new option --skip-render will be added to the deploy command. skaffold deploy --skip-render only deploy the kubernetes manifests without first rendering them. Deploy can also take manifests in GSC.

@ChrisGe4
Copy link
Contributor Author

@nkubala Please review when you get a chance.

@codecov
Copy link

codecov bot commented Jun 11, 2020

Codecov Report

Merging #4316 into master will decrease coverage by 0.03%.
The diff coverage is 61.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4316      +/-   ##
==========================================
- Coverage   71.77%   71.73%   -0.04%     
==========================================
  Files         324      324              
  Lines       12499    12540      +41     
==========================================
+ Hits         8971     8996      +25     
- Misses       2958     2972      +14     
- Partials      570      572       +2     
Impacted Files Coverage Δ
pkg/skaffold/config/options.go 100.00% <ø> (ø)
pkg/skaffold/deploy/util.go 60.97% <30.76%> (-2.92%) ⬇️
cmd/skaffold/app/cmd/deploy.go 66.66% <50.00%> (-1.91%) ⬇️
pkg/skaffold/deploy/kubectl.go 69.04% <81.81%> (+1.26%) ⬆️
cmd/skaffold/app/cmd/dev.go 82.85% <0.00%> (-3.81%) ⬇️
pkg/skaffold/build/custom/script.go 52.08% <0.00%> (-1.11%) ⬇️
pkg/skaffold/deploy/resource/deployment.go 74.48% <0.00%> (-0.77%) ⬇️
pkg/skaffold/debug/transform_nodejs.go 80.86% <0.00%> (-0.33%) ⬇️
pkg/diag/validator/pod.go 1.28% <0.00%> (-0.07%) ⬇️
pkg/skaffold/debug/transform_jvm.go 94.62% <0.00%> (-0.06%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fddc954...580d492. Read the comment docs.

@ChrisGe4
Copy link
Contributor Author

I run the failed test locally and they both succeeded.

for the failed integration test, I run deploy --images index.docker.io/library/busybox:1 --default-repo=

I got the following output from kubectl describe deployment kustomize-test

Name:                   kustomize-test
Namespace:              default
CreationTimestamp:      Mon, 15 Jun 2020 01:46:56 -0400
Labels:                 app=kustomize-test
                        app.kubernetes.io/managed-by=skaffold-unknown
                        skaffold.dev/builder=local
                        skaffold.dev/cleanup=true
                        skaffold.dev/deployer=kustomize
                        skaffold.dev/docker-api-version=1.40
                        skaffold.dev/run-id=4493d96b-2fd5-4e5b-aea4-b1fb7e23cb22
                        skaffold.dev/tag-policy=git-commit
Annotations:            deployment.kubernetes.io/revision: 1
                        kubectl.kubernetes.io/last-applied-configuration:
                          {"apiVersion":"apps/v1","kind":"Deployment","metadata":{"annotations":{},"labels":{"app":"kustomize-test","app.kubernetes.io/managed-by":"...
Selector:               app=kustomize-test
Replicas:               1 desired | 1 updated | 1 total | 1 available | 0 unavailable
StrategyType:           RollingUpdate
MinReadySeconds:        0
RollingUpdateStrategy:  25% max unavailable, 25% max surge
Pod Template:
  Labels:  app=kustomize-test
           app.kubernetes.io/managed-by=skaffold-unknown
           skaffold.dev/builder=local
           skaffold.dev/cleanup=true
           skaffold.dev/deployer=kustomize
           skaffold.dev/docker-api-version=1.40
           skaffold.dev/run-id=4493d96b-2fd5-4e5b-aea4-b1fb7e23cb22
           skaffold.dev/tag-policy=git-commit
  Containers:
   kustomize-test:
    Image:      index.docker.io/library/busybox:1
    Port:       <none>
    Host Port:  <none>
    Command:
      sleep
      3600
    Environment:  <none>
    Mounts:       <none>
  Volumes:        <none>
Conditions:
  Type           Status  Reason
  ----           ------  ------
  Available      True    MinimumReplicasAvailable
  Progressing    True    NewReplicaSetAvailable
OldReplicaSets:  <none>
NewReplicaSet:   kustomize-test-6c88b65c94 (1/1 replicas created)
Events:
  Type    Reason             Age   From                   Message
  ----    ------             ----  ----                   -------
  Normal  ScalingReplicaSet  70s   deployment-controller  Scaled up replica set kustomize-test-6c88b65c94 to 1

Not sure why got

   TestDeploy: deploy_test.go:81: string differ (-got, +want):   string(
        - 	"index.docker.io/library/busybox",
        + 	"index.docker.io/library/busybox:1",
          )

As to the failed unit test, it takes 72s to run it locally, but the timeout is set to 60s on the test server. Could be the cause of the failure?

ChrisGe4 added 4 commits June 17, 2020 21:24
…d_deploy_1

� Conflicts:
�	cmd/skaffold/app/cmd/deploy.go
�	pkg/skaffold/config/options.go
�	pkg/skaffold/deploy/kubectl.go
@ChrisGe4
Copy link
Contributor Author

I rerun those failed checks locally without seeing any error.

@tejal29 tejal29 self-assigned this Jun 18, 2020
Copy link
Contributor

@tejal29 tejal29 left a comment

Choose a reason for hiding this comment

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

The code looks good and working.
Few nits.

@ChrisGe4 ChrisGe4 requested a review from a team as a code owner June 19, 2020 19:21
@tejal29 tejal29 added the kokoro:run runs the kokoro jobs on a PR label Jun 22, 2020
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jun 22, 2020
@tejal29 tejal29 merged commit aa43689 into GoogleContainerTools:master Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants