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

Add 'skaffold apply' command #5543

Merged
merged 8 commits into from Mar 17, 2021

Conversation

nkubala
Copy link
Contributor

@nkubala nkubala commented Mar 12, 2021

This change adds a new command, skaffold apply, which accepts one or more hydrated Kubernetes manifests and uses them to create resources on the cluster.

skaffold apply does not perform any build actions (skaffold build), and does not perform any configuration management (skaffold render): it instead blindly consumes whatever Kubernetes configuration it is passed and sends it to kubectl. It will, however, monitor resource status through the built-in status-check mechanism.

This command unlocks the ability for Skaffold to consume its own output from skaffold render:

➜ (getting-started) skaffold render --output render.yaml

➜  (getting-started) cat render.yaml 
apiVersion: v1
kind: Pod
metadata:
  name: getting-started
  namespace: default
spec:
  containers:
  - image: skaffold-example:99e5f4d45e86d2aa180ebb6d382015d43383fef1ed7efcfff1a61fd205421479
    name: getting-started

➜ (getting-started) skaffold apply render.yaml
 - pod/getting-started created
Waiting for deployments to stabilize...
Deployments stabilized in 43.504125ms

Skaffold labels skaffold.dev/run-id and kubernetes.io/managed-by will be added to all deployed resources since they are required by the status checker. cc #5542

To be merged after #5541 - please review that PR first, and this change will be rebased for ease of review once that PR is merged.

Part 2 / Fixes #4856

@nkubala nkubala requested a review from a team as a code owner March 12, 2021 19:57
@google-cla google-cla bot added the cla: yes label Mar 12, 2021
@nkubala nkubala changed the title Apply command Add 'skaffold apply' command Mar 12, 2021
@nkubala nkubala force-pushed the apply-command branch 2 times, most recently from 93620ed to e72ebcc Compare March 12, 2021 20:14
@codecov
Copy link

codecov bot commented Mar 12, 2021

Codecov Report

Merging #5543 (657a131) into master (ef86216) will decrease coverage by 0.15%.
The diff coverage is 23.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5543      +/-   ##
==========================================
- Coverage   71.22%   71.07%   -0.16%     
==========================================
  Files         400      402       +2     
  Lines       14934    15055     +121     
==========================================
+ Hits        10637    10700      +63     
- Misses       3510     3563      +53     
- Partials      787      792       +5     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/flags.go 88.88% <ø> (+0.95%) ⬆️
pkg/skaffold/config/options.go 100.00% <ø> (ø)
pkg/skaffold/deploy/kubectl/cli.go 87.50% <ø> (ø)
pkg/skaffold/runner/apply.go 0.00% <0.00%> (ø)
pkg/skaffold/runner/new.go 63.13% <0.00%> (ø)
pkg/skaffold/runner/runner.go 0.00% <ø> (ø)
cmd/skaffold/app/cmd/apply.go 25.00% <25.00%> (ø)
cmd/skaffold/app/cmd/commands.go 81.25% <28.57%> (-9.00%) ⬇️
pkg/skaffold/deploy/kubectl/kubectl.go 65.18% <50.00%> (-1.48%) ⬇️
cmd/skaffold/app/cmd/cmd.go 63.29% <100.00%> (+0.23%) ⬆️
... and 37 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 ef86216...657a131. Read the comment docs.

}, doApply)
}

func doApply(ctx context.Context, out io.Writer, args []string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should you allow glob arguments like kubectl deployer currently does?

Copy link
Collaborator

Choose a reason for hiding this comment

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

should we support urls?

Copy link
Collaborator

Choose a reason for hiding this comment

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

should we support reading from STDIN so I could do this 😅:
In examples/multi-config-microservices

skaffold diagnose --yaml-only | skaffold render -f - | skaffold apply -

Copy link
Contributor Author

Choose a reason for hiding this comment

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

haha yes these are all great suggestions 😄 i think i'm fine with supporting all 3 of these, but how about i add them in a follow up PR? they seem like QOL enhancements to this feature

@nkubala
Copy link
Contributor Author

nkubala commented Mar 12, 2021

forgot to mention, docs are on their way

Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

I removed some passive voice

docs/content/en/docs/workflows/ci-cd.md Outdated Show resolved Hide resolved
docs/content/en/docs/workflows/ci-cd.md Show resolved Hide resolved
docs/content/en/docs/workflows/ci-cd.md Outdated Show resolved Hide resolved
docs/content/en/docs/workflows/ci-cd.md Outdated Show resolved Hide resolved
docs/content/en/docs/workflows/ci-cd.md Show resolved Hide resolved
docs/content/en/docs/workflows/ci-cd.md Outdated Show resolved Hide resolved
docs/content/en/docs/workflows/ci-cd.md Outdated Show resolved Hide resolved
docs/content/en/docs/workflows/ci-cd.md Outdated Show resolved Hide resolved
docs/content/en/docs/workflows/ci-cd.md Outdated Show resolved Hide resolved
nkubala and others added 5 commits March 17, 2021 09:51
Co-authored-by: Brian de Alwis <bsd@acm.org>
Co-authored-by: Brian de Alwis <bsd@acm.org>
Co-authored-by: Brian de Alwis <bsd@acm.org>
Co-authored-by: Brian de Alwis <bsd@acm.org>
@nkubala
Copy link
Contributor Author

nkubala commented Mar 17, 2021

CI is failing because of a docker rate limit issue:

RUN hack/test-generated-proto.sh
Sending build context to Docker daemon  435.8kB
Step 1/33 : FROM golang:1.13.4-alpine3.10 AS generate-files
1.13.4-alpine3.10: Pulling from library/golang
anyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit
Unable to find image 'gen-proto:latest' locally
docker: Error response from daemon: pull access denied for gen-proto, repository does not exist or may require 'docker login'.
See 'docker run --help'.
Generated proto files aren't updated. Please run ./hack/generate-proto.sh
FAILED hack/test-generated-proto.sh

@nkubala
Copy link
Contributor Author

nkubala commented Mar 17, 2021

codecov is reporting some very strange results: https://app.codecov.io/gh/GoogleContainerTools/skaffold/compare/5543/changes

going to self-merge since the rest of CI is green.

@nkubala nkubala merged commit 5bf0323 into GoogleContainerTools:master Mar 17, 2021
@nkubala nkubala deleted the apply-command branch June 17, 2021 18:25
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.

Accept render output in skaffold deploy
3 participants