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

Remove some unused labels. #1190

Merged
merged 1 commit into from
Dec 19, 2019
Merged

Remove some unused labels. #1190

merged 1 commit into from
Dec 19, 2019

Conversation

porridge
Copy link
Member

What this PR does / why we need it:

  • remove the "app: kudo-manager" from CRDs, since it is not really used
    by anything, and will be hard to convince controller-gen to generate
    it when we switch to it
  • remove the controller-tools.k8s.io label, (which is not even used by
    controller-tools upstream any more) from EVERYWHERE

This is another baby step towards #862

Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not really rejecting per say but I need to understand something first before I can approve :)

So looking at the labels in our current init, this is how statefulset for manager looks like

  selector:
    matchLabels:
      app: kudo-manager
      control-plane: controller-manager

if we remove app from there, what will actually remain? How will the statefulset select the pods? I don't think that just control-plane: controller-manager is a good idea.

@porridge porridge changed the base branch from crd-diff to master December 17, 2019 11:02
- remove the "app: kudo-manager" from CRDs, since it is not really used
  by anything, and will be hard to convince controller-gen to generate
  it when we switch to it
- remove the controller-tools.k8s.io label, (which is not even used by
  controller-tools upstream any more) from EVERYWHERE
@porridge
Copy link
Member Author

@alenkacz note that I'm not removing the app label from anything but CRDs.
I hoped that the PR description makes that clear, please let me know if you think it can be improved somehow.

Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's only CRDs and tests are passing, it should be fine :)

@porridge porridge merged commit 59f9ec9 into master Dec 19, 2019
@porridge porridge deleted the crd-rm-labels branch December 19, 2019 06:48
ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020
- remove the "app: kudo-manager" from CRDs, since it is not really used
  by anything, and will be hard to convince controller-gen to generate
  it when we switch to it
- remove the controller-tools.k8s.io label, (which is not even used by
  controller-tools upstream any more) from EVERYWHERE

Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants