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

add deprecated warning for node beta labels in pv/sc/rc/csi storage capacity #108554

Merged
merged 3 commits into from Aug 2, 2022

Conversation

pacoxu
Copy link
Member

@pacoxu pacoxu commented Mar 7, 2022

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

  • PersistentVolumeSpec
    • deprecated node labels in volume node affinity: pvSpec.NodeAffinity.Required.NodeSelectorTerms[n].MatchExpressions[m].Key
  • StorageClass
    • deprecated node labels: allowedTopologies.matchLabelExpressions.key ?
  • CSIStorageCapacity
    • deprecated node labels: nodeTopology.matchExpressions[m].Key
  • RuntimeClass
    • deprecated node labels: scheduling. nodeSelector
  • UT

Which issue(s) this PR fixes:

xref #94626 & #100243

Special notes for your reviewer:

[root@paco ~]# kubectl create -f pv.yaml --dry-run=server --v=6
I1021 13:58:54.985426   22875 loader.go:374] Config loaded from file:  /etc/kubernetes/admin.conf
I1021 13:58:55.052054   22875 round_trippers.go:553] GET https://10.6.177.41:6443/openapi/v2?timeout=32s 200 OK in 65 milliseconds
I1021 13:58:55.234544   22875 round_trippers.go:553] POST https://10.6.177.41:6443/api/v1/persistentvolumes?dryRun=All&fieldManager=kubectl-create&fieldValidation=Strict 201 Created in 6 milliseconds
Warning: spec.nodeAffinity.required.nodeSelectorTerms[0].matchExpressions[0].key: beta.kubernetes.io/os is deprecated since v1.14; use "kubernetes.io/os" instead
persistentvolume/foo-pv-1 created (server dry run)
[root@paco ~]# cat pv.yaml
apiVersion: v1
kind: PersistentVolume
metadata:
  name: foo-pv-1
spec:
  storageClassName: ""
  accessModes:
  - ReadWriteOnce
  capacity:
    storage: 5Gi
  nfs:
    path: /tmp
    server: 172.17.0.2
  nodeAffinity:
    required:
      nodeSelectorTerms:
      - matchExpressions:
        - key: beta.kubernetes.io/os
          operator: In
          values:
          - linux 

Does this PR introduce a user-facing change?

add a deprecated warning for node beta label usage in pv/sc/rc and csi storage capacity

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Mar 7, 2022
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 7, 2022
@pacoxu pacoxu changed the title add pv deprecated label using warning for node affinity add deprecated warning for node beta labels in pv/sc/rc/csi storage capacity Mar 7, 2022
@pacoxu pacoxu marked this pull request as ready for review March 9, 2022 01:17
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2022
@pacoxu
Copy link
Member Author

pacoxu commented Mar 9, 2022

/sig api-machinery

@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 9, 2022
@pacoxu pacoxu marked this pull request as draft March 9, 2022 01:20
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2022
@pacoxu pacoxu marked this pull request as ready for review March 9, 2022 01:24
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2022
@pacoxu
Copy link
Member Author

pacoxu commented May 13, 2022

Should someone review this pr again for v1.25? @liggitt

pkg/api/persistentvolume/util.go Outdated Show resolved Hide resolved
pkg/api/node/util.go Outdated Show resolved Hide resolved
pkg/api/persistentvolume/util.go Outdated Show resolved Hide resolved
pkg/api/persistentvolume/util_test.go Outdated Show resolved Hide resolved
pkg/api/node/util.go Outdated Show resolved Hide resolved
pkg/api/node/util.go Outdated Show resolved Hide resolved
pkg/api/storage/util.go Outdated Show resolved Hide resolved
pkg/api/storage/util.go Outdated Show resolved Hide resolved
@liggitt
Copy link
Member

liggitt commented May 24, 2022

sorry for the delay, looking forward to getting this in place in 1.25

@pacoxu
Copy link
Member Author

pacoxu commented Jun 21, 2022

kindly ping @liggitt
Do you have time a look?

@liggitt
Copy link
Member

liggitt commented Jun 21, 2022

Yes, picking this back up after 1.25 enhancements freeze.

@pacoxu
Copy link
Member Author

pacoxu commented Jul 20, 2022

ack

@liggitt liggitt self-assigned this Jul 28, 2022
@liggitt liggitt added the api-review Categorizes an issue or PR as actively needing an API review. label Jul 28, 2022
@liggitt liggitt added this to Assigned in API Reviews Jul 28, 2022
@pacoxu
Copy link
Member Author

pacoxu commented Aug 1, 2022

@liggitt do you have time to take a look at this again? Thanks.

@liggitt
Copy link
Member

liggitt commented Aug 2, 2022

/lgtm
/approve

@liggitt liggitt moved this from Assigned to API review completed, 1.25 in API Reviews Aug 2, 2022
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 2, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, pacoxu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 2, 2022
@k8s-ci-robot k8s-ci-robot merged commit 9ef16e7 into kubernetes:master Aug 2, 2022
SIG Node PR Triage automation moved this from Needs Reviewer to Done Aug 2, 2022
@sftim
Copy link
Contributor

sftim commented Aug 10, 2022

This is a change that results in a different behavior as experienced by end users. I recommend adding a changelog entry.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Aug 10, 2022
@pacoxu
Copy link
Member Author

pacoxu commented Aug 10, 2022

This is a change that results in a different behavior as experienced by end users. I recommend adding a changelog entry.

OK. I used the title.

@sftim
Copy link
Contributor

sftim commented Aug 10, 2022

Thanks @pacoxu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Status: API review completed, 1.25
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

6 participants