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

Use shorter label namespace #778

Closed
marquiz opened this issue Feb 17, 2022 · 18 comments
Closed

Use shorter label namespace #778

marquiz opened this issue Feb 17, 2022 · 18 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@marquiz
Copy link
Contributor

marquiz commented Feb 17, 2022

What would you like to be added:

I'd like to move to a shorter label namespace (e.g. nfd.k8s.io/ or feature.node.k8s.io/) before nfd v1.0. I'm not sure what would be the least painful way there (or if it even is worth it).

One path could be:

  1. Allow the shorter namespace. For custom labels only
  2. Have an option to switch to the shorter namespace for all labels (the old long namespaces would still be the default)
  3. Make the new namespace the default for built-in labels. Have an option to use the old one instead
  4. Drop the compatibility mode (no hurry with this)

Why is this needed:

Usability

@marquiz marquiz added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 17, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 18, 2022
@zvonkok
Copy link
Contributor

zvonkok commented May 18, 2022

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 18, 2022
@marquiz marquiz modified the milestone: v.0.12.0 Jun 30, 2022
@marquiz
Copy link
Contributor Author

marquiz commented Jun 30, 2022

@zvonkok @ArangoGutierrez what do you think about this? Is it too disruptive?

OTOH we can start implementing this with the two first steps (allow nfd.k8s.io and create a flag for turning that on for all labels). This gives us time to experiment and get feedback based on which we can do the switchover of back off.

@ArangoGutierrez
Copy link
Contributor

Agree that this should be optional for some time. Many workloads might have hard coded the current schema.

Question: can we use k8s when becoming a 1.0 API?

@marquiz
Copy link
Contributor Author

marquiz commented Jun 30, 2022

I think labels this is ok already now. The long current one (feature.node.kubernetes.io) was agreed in SIG node years ago.

@ArangoGutierrez
Copy link
Contributor

/lgtm

@fmuyassarov
Copy link
Member

fmuyassarov commented Sep 8, 2022

I think it is better if we keep discussion notes here. So, I will copy paste my comments here from the #881.

This is a breaking change, and usually in k8s for such a change it takes ~3-4 releases to remove a feature. Example: rename "node-role.kubernetes.io/master" node-role.kubernetes.io/master label & taint renaming in Kubeadm, they had both labels in parallel that gave time to users to switch to the new format. And perhaps we can follow the same path?

For example,

Release - v0.12.X

  1. introduce a short label and a flag to switch between long and short labels
  2. set the flag to use long label by default
  3. announce to community/users to adapt to use the short label
  4. get community/users feedback

Release - v0.13.X

  1. turn on both short and long labels by default but still have an option to disable them
  2. get community/users feedback

Release - v0.14.X

  1. Turn off long label by default but still have an option to enable it

Release - v0.15.X

  1. Remove the long label entirely from the codebase and keep only short label
  2. Remove the flag to switch between short and long labels

I'm not sure about the estimated time between MINOR releases in NFD project, but if you think it is too short that we take above steps in every MINOR release, then of course we can extend it. And actually better if we get users feedback on that.

Patch for the first step is ready for review in #881 and if you folks agree with the plan then let's move on. Any comments, thoughts, concerns are welcome. I will also spread the word on the slack.

@fmuyassarov
Copy link
Member

/assign

@marquiz
Copy link
Contributor Author

marquiz commented Sep 8, 2022

I think it is better if we keep discussion notes here.

Yup 👍

@ArangoGutierrez
Copy link
Contributor

What if for a next step we make it optional?
in the CRD add an entry short namespace :yes/no

thoughts?

@fmuyassarov
Copy link
Member

What if for a next step we make it optional? in the CRD add an entry short namespace :yes/no

thoughts?

As per discussion on the slack, I was thinking of introducing a flag to turn on/off the new label but I'm also fine with the CRD as well. TBH, I don't have a strong opinion which was one is better.

@marquiz
Copy link
Contributor Author

marquiz commented Sep 12, 2022

What CRD are you referring to? If it's operator then sure. But this needs to be a single config option in nfd-master (i.e. cluster-wide)

@ArangoGutierrez
Copy link
Contributor

yes, I agree, it should be a flag. see the slack link above

@fmuyassarov fmuyassarov mentioned this issue Dec 8, 2022
22 tasks
@marquiz marquiz removed this from the v.0.12.0 milestone Dec 8, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 8, 2023
@ArangoGutierrez
Copy link
Contributor

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 8, 2023
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 6, 2023
@marquiz
Copy link
Contributor Author

marquiz commented Jun 22, 2023

I think we're not doing this for now. Too much hassle, especially for the end user. Ref discussion in #881
/close

@k8s-ci-robot
Copy link
Contributor

@marquiz: Closing this issue.

In response to this:

I think we're not doing this for now. Too much hassle, especially for the end user. Ref discussion in #881
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants