Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Use "app" label in selectors #17

Closed
wants to merge 1 commit into from
Closed

Conversation

kupson
Copy link

@kupson kupson commented Mar 23, 2019

Fixes issue #16 - Deployment's selector should be immutable

Scenario:

  • Deployment is created with kube.libsonnet, CI process adds additional labels
    e.g. "ci-job-id"
  • Deployment is updated with the same process but the CI process was updated to
    add new label e.g. "ci-cluster"
  • kubectl apply fails as the selector field is immutable

This adds an additional label "app" to Pod, Deployment, StatefulSet, DaemonSet,
Job and CronJob objects and use it when those objects needs to be referred by
selectors.

Fixes issue bitnami#16 - Deployment's selector should be immutable

Scenario:
- Deployment is created with kube.libsonnet, CI process adds additional labels
  e.g. "ci-job-id"
- Deployment is updated with the same process but the CI process was updated to
  add new label e.g. "ci-cluster"
- kubectl apply fails as the selector field is immutable

This adds an additional label "app" to Pod, Deployment, StatefulSet, DaemonSet,
Job and CronJob objects and use it when those objects needs to be referred by
selectors.
@kupson kupson requested review from anguslees and jjo as code owners March 23, 2019 07:36
@anguslees
Copy link
Contributor

Thanks for the suggestion! Using "app" is probably a common situation for people migrating an existing config to kube.libsonnet, so it's good to see what we can do to make this easier.

There are a couple of conceptually-separate changes here, which I'd like to discuss separately:

1. Rename name=foo label to app=foo

I don't think we can do (1) as it is in this PR 😞. I know a lot of the rest of the community uses "app=name", but this convention is purely cosmetic, and would introduce a change of labels to all existing kube.libsonnet users (triggering exactly the issue this PR is attempting to avoid!).

If I had a time machine, I would probably choose to use app=foo instead, although even then I would question it tbh. In the low-level automated context we're talking about in kube.libsonnet we don't know what the "app" in question is - all we know is the name given to the k8s object (eg: we know "elasticsearch-master-node", not "elasticsearch"). The introduction of Application objects confuses this even further if the app labels don't match the same grouping hierarchy.

See also below.

2. Introduce a new _AppObject() "constructor"

(2) is unnecessary, if we drop (1).

As a general pattern however, creating new constructors doesn't compose well (what if we had several special tweaks we wanted to apply to this Object?), and "mixins" tend to work better.

ie: Consider the following. You can chain multiple "mixins" to add additional modifiers:

{
  applabels:: {
    local name = self.metadata.name,
    metadata+: {labels: {app: name}},
  },

  Deployment(name):: $._Object(name) + $.applabels + {
    // as normal ...
  },
}

3. Change selector to a subset of the labels (just {app: value})

Separating the top-level Deployment labels from the low-level Pod label/selector is probably a good idea, for the reasons you raise. I liked the way the selector was "exported" from the object in question in the previous version (by just assuming that it was all metadata.labels), rather than having everything else "know" that the appropriate choice is {app=target.labels.app}. It would be good to get some of that back - probably by adding a hidden selector field to all objects, and then using that everywhere we need a cross-object selector...

Let me think about that for a bit 😛

Note for (1)

If a local site was using an "app=foo" convention, and would like to continue using this convention with kube.libsonnet then they can easily do this with a local customisation of kube.libsonnet that has something like this PR in a separate "overlay" file:

// Call this "myorganisation.libsonnet" or similar
local kube = import "kube.libsonnet";
local applabels = {
  metadata+: {labels: {app: $.metadata.name}},
};

{
  Deployment(name):: kube.Deployment(name) + applabels {
    // .. and any other local policies
  },
  StatefulSet(name):: kube.Deployment(name) + applabels,
  // and so on, for other modified types.
}

.. and then of course use it as usual:

// Example usage
local myorg = import "myorganisation.libsonnet";
{
  example: myorg.Deployment(name) {
    spec+: {
      // ... as usual
    },
  },
}

@kupson
Copy link
Author

kupson commented Apr 2, 2019

Thank you for the review and ideas how to implement our fixes in a better way. I'll close the pull request as the issue needs to be fixed in a different way.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants