Disable deploying global resources by default #567
Conversation
dturn
force-pushed
the
global-deploy
branch
2 times, most recently
from
372a56b
to
3111ee0
Compare
dturn
force-pushed
the
global-deploy
branch
from
3111ee0
to
3548995
Compare
dturn
changed the title
dturn
force-pushed
the
global-deploy
branch
from
1ff1843
to
1cdb577
Compare
dturn
force-pushed
the
global-deploy
branch
5 times, most recently
from
ffbc3cd
to
27ebe38
Compare
| inst.type = definition["kind"] | ||
| type = definition["kind"] | ||
| inst = if globals.include?(type.downcase) | ||
| KubernetesDeploy::GlobalKubernetesResource.new(**opts) |
There was a problem hiding this comment.
What if instead of having a new class, we make global settable, and handle it like we do type below? I think might be a bit simpler overall.
There was a problem hiding this comment.
I went back and forth on this couldn't decide which was better. This felt more future proof, though I can't think of any special behavior we'd want from a GlobalResource., but the approach of not using a new class was definitely simpler. I'll try the other approach and see how it goes.
There was a problem hiding this comment.
🤔 If we considered server resource lists part of the task's global operating context and put them into our new task config object, then we'd have a third option that might read even more straightforwardly: inside the normal superclass, we could do essentially def global?; @task_config.global_resource_kinds.include?(kind); end.
There was a problem hiding this comment.
I think that makes sense to do in #572.
| @@ -62,7 +62,7 @@ def deploy_raw_fixtures(set, wait: true, bindings: {}, subset: nil) | |||
|
|
|||
| def deploy_dirs_without_profiling(dirs, wait: true, allow_protected_ns: false, prune: true, bindings: {}, | |||
| sha: "k#{SecureRandom.hex(6)}", kubectl_instance: nil, max_watch_seconds: nil, selector: nil, | |||
| protected_namespaces: nil, render_erb: true) | |||
| protected_namespaces: nil, render_erb: true, allow_globals: true) | |||
There was a problem hiding this comment.
Is the plan to change the default when we remove KubernetesDeploy?
dturn
force-pushed
the
global-deploy
branch
from
a7e39a0
to
8f2a93d
Compare
dturn
force-pushed
the
global-deploy
branch
3 times, most recently
from
1bbc1cb
to
bd32085
Compare
Co-Authored-By: Katrina Verey <katrina.verey@shopify.com>
…g values in some columns so relying on whitespace as a delimiter isn't sufficent
What are you trying to accomplish with this PR?
As part of the 1.0 push we are going to require a new command to deploy global resources #522
This is the first step, preventing
krane deployfrom deploying global resources. This does maintain backwards compatibility with kubernetes-deploy.How is this accomplished?
Fetch global resources from the cluster and warn/raise if any are part of the deploy set.
What could go wrong?
We miss global resources or this change upsets people.