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

[regression] Skaffold @ HEAD (in no releases) currently adds a namespace to all rendered manifests #8186

Closed
aaron-prindle opened this issue Dec 2, 2022 · 5 comments · Fixed by #8312
Labels
kind/regression kind/todo implementation task/epic for the skaffold team
Milestone

Comments

@aaron-prindle
Copy link
Contributor

aaron-prindle commented Dec 2, 2022

With #8105 merged, currently skaffold will always add a namespace to rendered yamls:

aprindle@aprindle-ssd ~/skaffold/examples/getting-started  [main]$ skaffold render
apiVersion: v1
kind: Pod
metadata:
  name: getting-started
  namespace: default # <--- this is set
spec:
  containers:
  - image: skaffold-example:v2.0.2-45-g130df30ae
    name: getting-started

This is an issue as skaffold cannot then use rendered manifests to deploy to multiple namespaces as most/all deployers will not "overwrite" a namespace set in the yaml config (will error stating such).

Behavior should be:
When no namespace is explicitly set, no namespace should be added to allow for deploying to multiple namespaces

@aaron-prindle
Copy link
Contributor Author

aaron-prindle commented Jan 12, 2023

I am still seeing this after reverting #8124, not sure if there is something wrong in my local env or this is unresolved. Re-opening

@aaron-prindle
Copy link
Contributor Author

There was some confusion here, currently with that PR reverted I believe users get the desired functionality using offline=true but I think we should likely change the default behaviour here to be the same as what is currently only used for offline=true. Additionally should document this behavior better and add golden paths for how to do the skaffold CI/CD flow of build -> render -> deploy

@aaron-prindle
Copy link
Contributor Author

Going to create a new issue that better describes this problem (vs reverting this issue which mentions reverting the PR)

@jleonar
Copy link

jleonar commented Jan 12, 2023

@aaron-prindle Does this mean default doesn't get added to all manifests where the namespace isn't specified in the yamls? My use case is sorta what you mention, we build in one environment and publish artifacts. Then use render -> deploy in our prod environment. This default getting added will break for us since we use manifests that don't specify a namespace.

@aaron-prindle
Copy link
Contributor Author

aaron-prindle commented Jan 12, 2023

@jleonar can you repost your comment in the new issue tracking this:
#8318

I will answer there when you post there to make the convo more visible/coherent to other readers, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/regression kind/todo implementation task/epic for the skaffold team
Projects
None yet
2 participants