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

skaffold build should support a --push flag to override build.local.push #5667

Closed
briandealwis opened this issue Apr 12, 2021 · 4 comments · Fixed by #5708
Closed

skaffold build should support a --push flag to override build.local.push #5667

briandealwis opened this issue Apr 12, 2021 · 4 comments · Fixed by #5708
Assignees
Labels
good first issue Good for newcomers help wanted We would love to have this done, but don't have the bandwidth, need help from contributors kind/feature-request priority/p2 May take a couple of releases

Comments

@briandealwis
Copy link
Member

The container-debug-support images uses skaffold build to build and test images. These builds don't have a cluster available and so a plain skaffold build causes Skaffold to attempt to build and push images, which fails. So we explicitly configure build.local.push: false so that the images are only built to the local Docker daemon.

@tejal29 tejal29 added the priority/p2 May take a couple of releases label Apr 12, 2021
@tejal29
Copy link
Member

tejal29 commented Apr 12, 2021

Discussion notes:

  1. skaffold build should not run tests
  2. skaffold test with --build-artifacts not run build.
    • Current behavior: skaffold test should run build only if no --build-artifacts is present
  3. Adding a command a line skaffold build --push and respect --push.

@tejal29 tejal29 added help wanted We would love to have this done, but don't have the bandwidth, need help from contributors good first issue Good for newcomers and removed triage/discuss Items for discussion labels Apr 12, 2021
@aaron-prindle aaron-prindle self-assigned this Apr 14, 2021
@aaron-prindle
Copy link
Contributor

aaron-prindle commented Apr 19, 2021

To clarify how the --push flag should work, does this mean that skaffold build now defaults to not pushing images w/o the --push flag (and also no config set)? Or does it just have precendence over the config setting - build.local.push: false w/ no changes to the default?

aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 19, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml pushImages configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by one command, similar to #GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 19, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by one command, similar to #GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 19, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
@gsquared94
Copy link
Collaborator

Right now, if build.local.push is explicitly set then it uses that value, otherwise it tries to infer:

  • If the cluster is minikube (we run minikube profile command, check a few heuristics), don't push
  • If the cluster is kind or k3d, don't push; unless the cluster-image-load ( a global config property) is also disabled, in which case push
  • For all other clusters push

This approach does however have the unintuitive behavior of checking kube-context and cluster details (like minikube docker-env, etc.) even when the intention is to only build.

@gsquared94
Copy link
Collaborator

From OP's sentiment, I think they want to be able to say skaffold build --push=false and override whatever is set in the skaffold.yaml file.

aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 20, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 20, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 21, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 21, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 21, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 21, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 21, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 21, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 21, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 21, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 21, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 22, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 22, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 23, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 23, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 23, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 23, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 23, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 23, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
aaron-prindle added a commit to aaron-prindle/skaffold that referenced this issue Apr 24, 2021
What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
tejal29 pushed a commit that referenced this issue Apr 26, 2021
What is the problem being solved?
Fixes #5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to #4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted We would love to have this done, but don't have the bandwidth, need help from contributors kind/feature-request priority/p2 May take a couple of releases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants