rules_k8s icon indicating copy to clipboard operation
rules_k8s copied to clipboard

some rule attributes shadow the template data

Open ensonic opened this issue 8 years ago • 10 comments

k8s_object(kind) is the same as yaml(kind) k8s_object(namespace) is the same as yaml(metadata/namespace)

Replicating the on the k8s_object rule is IMHO not a good idea. This brings the danger that one specifies the different kind on the rule and in the template. Even worse one needs to repeat data if they are different from the default (e.g. namespace).

What is the advantage of having those on the rule in the first place?

Can we change the way this works: 1.) Print a warning if the value from the rule differs from the yaml/json 2.) If the value is not given in the rule, take it from the yaml/json, else use the default?

ensonic avatar Nov 27 '17 12:11 ensonic

re: kind

You can safely omit kind on rules, IIRC it is only used for bazel run :foo.describe, and that action will simply not be present if the kind is omitted.

I've been mulling the removal of both kind and describe for the reasons you give. Alternately, I would like to perform the validation you suggest, but haven't found the time.

re: namespace

This is an interesting intersection I hadn't yet considered. I don't think this is exactly the same as kind because I don't believe you may omit kind, but you can omit metadata.namespace. This leads me to believe that unlike kind the rules may also override the namespace, which wouldn't make sense for kind.

This makes me wonder if we're even doing the right thing when k8s_object omits namespace, but it is present in the yaml.

Perhaps the "right" solution would be to introduce a basic understanding of K8s-style resources into the resolver (or a version that operates at build time) and semantically update/create metadata.namespace instead of passing it on the command-line to kubectl.

mattmoor avatar Nov 27 '17 17:11 mattmoor

I agree that both attributes might need different treatment. I would prefer to keep the rules simple and configure everything in the template.

For kind, it is not critical since it is optional and we can simply omit it.

For namespace it would mean to not use the flag, if namespace is not set by the user (then the value from the template is used or 'default' if none is set). I could make a PR for this if you agree.

ensonic avatar Nov 28 '17 09:11 ensonic

Yes, I would be fine with that change in flag behavior.

mattmoor avatar Nov 28 '17 15:11 mattmoor

I don't see the benefit for repeating kind/namespace in the build file. It has became a huge burden to keep the files in sync. The k8s config file should be the only source of truth for what's being pushed.

As of the last patch, this is still not function properly. As the namespace attribute for k8s_object is default to "default", which makes it a required attribute in practice.

I'd suggest to drop the default values for namespace in k8s.bzl.

ashi009 avatar Dec 27 '17 05:12 ashi009

I'd suggest to drop the default values for namespace in k8s.bzl.

You means those: https://github.com/bazelbuild/rules_k8s/blob/master/k8s/object.bzl#L169 Makes sense, will do next year if you are not quicker with a PR.

ensonic avatar Dec 27 '17 13:12 ensonic

@ashi009 kind is optional, if you omit it the only thing you lose is :foo.describe. I have been mulling completely removing it.

I assume what you are after wrt namespace is respecting metadata.namespace, which because of the default value of "default" it is not respecting today?

mattmoor avatar Dec 27 '17 16:12 mattmoor

@ensonic Just sent a PR.

@mattmoor I defined the namespace in template, and omitted the namespace attribute in the rule. Which leads to the error that kubectl being called with --namespace=default which doesn't match the one in the config file.

ashi009 avatar Dec 28 '17 04:12 ashi009

The PR is merged, thanks!

mattmoor avatar Dec 28 '17 13:12 mattmoor

Just to add to this discussion, I agree the kind field is a bit confusing and also does not work if one has multiple kinds in one yaml file. Also why not just take the kind directly from the template on the .describe action? Also slightly separately, I have noticed that the .describe command does not run custom resolvers, making it not work with some of our templates.

On the namespace, I do agree here with @mattmoor that this is a different consideration and I actually like that one can specify that in the config and gets access to bazels stamping functionality. We want to deploy the same template often in different environments/namespaces, so having it defined in the bazel config makes this very declerative and I prefer it to what we had before.

Globegitter avatar Feb 08 '18 09:02 Globegitter

One issue I am running now into though, that I can use bazel to manage in which namespace a template gets deployed into, but it can not manage creation or deletion, so for that I would have to have a namespace template where I pass the namespace in separately i.e. having duplication. I would love to be able to fully manage namespaces with bazel/rules_k8s directly, i.e. have some way to create and delete namespace maybe with additional attributes on the rules. Something like:

k8s_object(
    ...
    namespace = {
        name: "some-namespace",
        createIfNotExists: True/False,
        removeOnDelete: False/False,
    }
)

Or maybe a new k8s_namespace rule that can be referenced on k8s_object, something like:

k8s_namespace(
    name = "mynamespace",
    createIfNotExists = True/False,
    removeOnDelete = True/False,
)

k8s_object(
    ...
    namespace = ":mynamespace"
)

@mattmoor Also, how does it work if I have two k8s_object that have a different namespace assigned and then I group them together in the same k8s_objects. Will the different namespaces be respected?

Globegitter avatar Apr 18 '18 13:04 Globegitter