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 "all" category from virtualmachineclusterinstancetypes/preferences #9536

Merged

Conversation

akalenyu
Copy link
Contributor

What this PR does / why we need it:
This results in k get all -n <namespace> returning those cluster-scoped resources.

$ oc get all -n openshift-config
Warning: kubevirt.io/v1 VirtualMachineInstancePresets is now deprecated and will be removed in v2.
NAME                                                                                AGE
virtualmachineclusterinstancetype.instancetype.kubevirt.io/cx1.2xlarge              5d22h
virtualmachineclusterinstancetype.instancetype.kubevirt.io/cx1.4xlarge              5d22h

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #
https://bugzilla.redhat.com/show_bug.cgi?id=2177763

Special notes for your reviewer:

Release note:

BugFix: virtualmachineclusterinstancetypes/preferences show up for get all -n <namespace>

…machineclusterpreferences

This results in `k get all -n <namespace>` returning those cluster-scoped resources.
```bash
$ oc get all -n openshift-config
Warning: kubevirt.io/v1 VirtualMachineInstancePresets is now deprecated and will be removed in v2.
NAME                                                                                AGE
virtualmachineclusterinstancetype.instancetype.kubevirt.io/cx1.2xlarge              5d22h
virtualmachineclusterinstancetype.instancetype.kubevirt.io/cx1.4xlarge              5d22h
```

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/XS labels Mar 29, 2023
@akalenyu
Copy link
Contributor Author

/cc @lyarwood

Copy link
Member

@lyarwood lyarwood left a comment

Choose a reason for hiding this comment

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

/area instancetype

I don't think this is correct, it drops the cluster wide resources from get all -A that we really don't want.

There has to be something else causing these resources to show up in the namespace, I'll try to look deeper now.

./cluster-up/kubectl.sh apply -f https://k8s.io/examples/admin/namespace-dev.yaml
./cluster-up/kubectl.sh apply -f examples/csmall.yaml -f examples/vm-cirros-csmall.yaml -n development
[..]
./cluster-up/kubectl.sh kustomize https://github.com/kubevirt/common-instancetypes.git | ./cluster-up/kubectl.sh apply -f -
[..]
./cluster-up/kubectl.sh get all -n development
selecting docker as container runtime
Warning: kubevirt.io/v1 VirtualMachineInstancePresets is now deprecated and will be removed in v2.
NAME                                          AGE   STATUS    READY
virtualmachine.kubevirt.io/vm-cirros-csmall   35s   Stopped   False

NAME                                                         AGE
virtualmachineinstancetype.instancetype.kubevirt.io/csmall   35s

./cluster-up/kubectl.sh get all -A
selecting docker as container runtime
Warning: kubevirt.io/v1 VirtualMachineInstancePresets is now deprecated and will be removed in v2.
NAMESPACE     NAME                                                READY   STATUS      RESTARTS   AGE
cdi           pod/cdi-apiserver-5b77c7f5bf-6s6jb                  1/1     Running     0          6m13s
cdi           pod/cdi-deployment-855f498c58-p2269                 1/1     Running     0          6m1s
cdi           pod/cdi-operator-f8bbdf8c5-fwncj                    1/1     Running     0          9m57s
cdi           pod/cdi-uploadproxy-d6ccf-vm2tj                     1/1     Running     0          6m1s
default       pod/local-volume-provisioner-8wlmv                  1/1     Running     0          11m
default       pod/snapshot-controller-0                           1/1     Running     0          11m
kube-system   pod/calico-kube-controllers-7bbc78ddf7-7c4g8        1/1     Running     0          11m
kube-system   pod/calico-node-f962h                               1/1     Running     0          11m
kube-system   pod/coredns-659bb6b7c8-chcx5                        1/1     Running     0          11m
kube-system   pod/coredns-659bb6b7c8-t5vr9                        1/1     Running     0          11m
kube-system   pod/etcd-node01                                     1/1     Running     1          11m
kube-system   pod/kube-apiserver-node01                           1/1     Running     1          11m
kube-system   pod/kube-controller-manager-node01                  1/1     Running     1          11m
kube-system   pod/kube-proxy-f4m8n                                1/1     Running     0          11m
kube-system   pod/kube-scheduler-node01                           1/1     Running     1          11m
kubevirt      pod/disks-images-provider-92wrw                     1/1     Running     0          6m24s
kubevirt      pod/virt-api-69d749dfd8-87pds                       1/1     Running     0          5m49s
kubevirt      pod/virt-controller-cc47d4f46-kfrrn                 1/1     Running     0          5m24s
kubevirt      pod/virt-controller-cc47d4f46-shx6n                 1/1     Running     0          5m24s
kubevirt      pod/virt-handler-r25lz                              1/1     Running     0          5m24s
kubevirt      pod/virt-operator-84997889cb-hfzrr                  1/1     Running     0          6m23s
kubevirt      pod/virt-operator-84997889cb-lk5tt                  1/1     Running     0          6m23s
rook-ceph     pod/csi-cephfsplugin-gmvtl                          3/3     Running     0          11m
rook-ceph     pod/csi-cephfsplugin-provisioner-5846cb8f8d-hr2lp   6/6     Running     0          11m
rook-ceph     pod/csi-rbdplugin-provisioner-984b4cddb-n82c9       6/6     Running     0          11m
rook-ceph     pod/csi-rbdplugin-zxz84                             3/3     Running     0          11m
rook-ceph     pod/rook-ceph-mgr-a-5dd6cd6f7d-qwg6v                1/1     Running     0          10m
rook-ceph     pod/rook-ceph-mon-a-8684459866-vdsh2                1/1     Running     0          11m
rook-ceph     pod/rook-ceph-operator-67b9bd5f47-d976t             1/1     Running     0          11m
rook-ceph     pod/rook-ceph-osd-0-54856f699-rhxw5                 1/1     Running     0          10m
rook-ceph     pod/rook-ceph-osd-prepare-node01-rmmzc              0/1     Completed   0          10m

NAMESPACE     NAME                                  TYPE        CLUSTER-IP       EXTERNAL-IP   PORT(S)                  AGE
cdi           service/cdi-api                       ClusterIP   10.96.191.180    <none>        443/TCP                  9m45s
cdi           service/cdi-prometheus-metrics        ClusterIP   10.105.95.80     <none>        8080/TCP                 9m41s
cdi           service/cdi-uploadproxy               ClusterIP   10.108.11.173    <none>        443/TCP                  9m39s
cdi           service/cdi-uploadproxy-nodeport      NodePort    10.103.141.124   <none>        443:31001/TCP            6m24s
default       service/kubernetes                    ClusterIP   10.96.0.1        <none>        443/TCP                  11m
kube-system   service/kube-dns                      ClusterIP   10.96.0.10       <none>        53/UDP,53/TCP,9153/TCP   11m
kubevirt      service/kubevirt-operator-webhook     ClusterIP   10.100.175.124   <none>        443/TCP                  5m52s
kubevirt      service/kubevirt-prometheus-metrics   ClusterIP   None             <none>        443/TCP                  5m52s
kubevirt      service/virt-api                      ClusterIP   10.97.212.42     <none>        443/TCP                  5m52s
kubevirt      service/virt-exportproxy              ClusterIP   10.101.58.115    <none>        443/TCP                  5m52s
rook-ceph     service/csi-cephfsplugin-metrics      ClusterIP   10.108.158.72    <none>        8080/TCP,8081/TCP        11m
rook-ceph     service/csi-rbdplugin-metrics         ClusterIP   10.106.129.30    <none>        8080/TCP,8081/TCP        11m
rook-ceph     service/rook-ceph-mgr                 ClusterIP   10.109.153.74    <none>        9283/TCP                 10m
rook-ceph     service/rook-ceph-mgr-dashboard       ClusterIP   10.101.33.185    <none>        7000/TCP                 10m
rook-ceph     service/rook-ceph-mon-a               ClusterIP   10.99.170.93     <none>        6789/TCP,3300/TCP        11m

NAMESPACE     NAME                                      DESIRED   CURRENT   READY   UP-TO-DATE   AVAILABLE   NODE SELECTOR            AGE
default       daemonset.apps/local-volume-provisioner   1         1         1       1            1           <none>                   11m
kube-system   daemonset.apps/calico-node                1         1         1       1            1           kubernetes.io/os=linux   11m
kube-system   daemonset.apps/kube-proxy                 1         1         1       1            1           kubernetes.io/os=linux   11m
kubevirt      daemonset.apps/disks-images-provider      1         1         1       1            1           <none>                   6m24s
kubevirt      daemonset.apps/virt-handler               1         1         1       1            1           kubernetes.io/os=linux   5m24s
rook-ceph     daemonset.apps/csi-cephfsplugin           1         1         1       1            1           <none>                   11m
rook-ceph     daemonset.apps/csi-rbdplugin              1         1         1       1            1           <none>                   11m

NAMESPACE     NAME                                           READY   UP-TO-DATE   AVAILABLE   AGE
cdi           deployment.apps/cdi-apiserver                  1/1     1            1           9m45s
cdi           deployment.apps/cdi-deployment                 1/1     1            1           9m41s
cdi           deployment.apps/cdi-operator                   1/1     1            1           9m57s
cdi           deployment.apps/cdi-uploadproxy                1/1     1            1           9m39s
kube-system   deployment.apps/calico-kube-controllers        1/1     1            1           11m
kube-system   deployment.apps/coredns                        2/2     2            2           11m
kubevirt      deployment.apps/virt-api                       1/1     1            1           5m49s
kubevirt      deployment.apps/virt-controller                2/2     2            2           5m24s
kubevirt      deployment.apps/virt-operator                  2/2     2            2           6m23s
rook-ceph     deployment.apps/csi-cephfsplugin-provisioner   1/1     1            1           11m
rook-ceph     deployment.apps/csi-rbdplugin-provisioner      1/1     1            1           11m
rook-ceph     deployment.apps/rook-ceph-mgr-a                1/1     1            1           10m
rook-ceph     deployment.apps/rook-ceph-mon-a                1/1     1            1           11m
rook-ceph     deployment.apps/rook-ceph-operator             1/1     1            1           11m
rook-ceph     deployment.apps/rook-ceph-osd-0                1/1     1            1           10m

NAMESPACE     NAME                                                      DESIRED   CURRENT   READY   AGE
cdi           replicaset.apps/cdi-apiserver-5b77c7f5bf                  1         1         1       6m14s
cdi           replicaset.apps/cdi-apiserver-8d89c949b                   0         0         0       9m45s
cdi           replicaset.apps/cdi-deployment-855f498c58                 1         1         1       6m1s
cdi           replicaset.apps/cdi-deployment-f56c9476f                  0         0         0       9m41s
cdi           replicaset.apps/cdi-operator-f8bbdf8c5                    1         1         1       9m57s
cdi           replicaset.apps/cdi-uploadproxy-6554867d98                0         0         0       9m39s
cdi           replicaset.apps/cdi-uploadproxy-d6ccf                     1         1         1       6m1s
kube-system   replicaset.apps/calico-kube-controllers-7bbc78ddf7        1         1         1       11m
kube-system   replicaset.apps/coredns-659bb6b7c8                        2         2         2       11m
kubevirt      replicaset.apps/virt-api-69d749dfd8                       1         1         1       5m49s
kubevirt      replicaset.apps/virt-controller-cc47d4f46                 2         2         2       5m24s
kubevirt      replicaset.apps/virt-operator-84997889cb                  2         2         2       6m23s
rook-ceph     replicaset.apps/csi-cephfsplugin-provisioner-5846cb8f8d   1         1         1       11m
rook-ceph     replicaset.apps/csi-rbdplugin-provisioner-984b4cddb       1         1         1       11m
rook-ceph     replicaset.apps/rook-ceph-mgr-a-5dd6cd6f7d                1         1         1       10m
rook-ceph     replicaset.apps/rook-ceph-mon-a-8684459866                1         1         1       11m
rook-ceph     replicaset.apps/rook-ceph-operator-67b9bd5f47             1         1         1       11m
rook-ceph     replicaset.apps/rook-ceph-osd-0-54856f699                 1         1         1       10m

NAMESPACE   NAME                                   READY   AGE
default     statefulset.apps/snapshot-controller   1/1     11m

NAMESPACE   NAME                                     COMPLETIONS   DURATION   AGE
rook-ceph   job.batch/rook-ceph-osd-prepare-node01   1/1           7s         10m

NAMESPACE   NAME                            AGE     PHASE
kubevirt    kubevirt.kubevirt.io/kubevirt   6m20s   Deployed

NAMESPACE     NAME                                          AGE     STATUS    READY
development   virtualmachine.kubevirt.io/vm-cirros-csmall   3m44s   Stopped   False

NAMESPACE     NAME                                                                         AGE
default       virtualmachineinstancetype.instancetype.kubevirt.io/cx1.2xlarge              3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/cx1.4xlarge              3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/cx1.8xlarge              3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/cx1.large                3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/cx1.medium               3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/cx1.xlarge               3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/gn1.2xlarge              3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/gn1.4xlarge              3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/gn1.8xlarge              3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/gn1.xlarge               3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/highperformance.large    3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/highperformance.medium   3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/highperformance.small    3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/m1.2xlarge               3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/m1.4xlarge               3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/m1.8xlarge               3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/m1.large                 3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/m1.xlarge                3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/n1.2xlarge               3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/n1.4xlarge               3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/n1.8xlarge               3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/n1.large                 3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/n1.medium                3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/n1.xlarge                3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/server.large             3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/server.medium            3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/server.micro             3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/server.small             3m12s
default       virtualmachineinstancetype.instancetype.kubevirt.io/server.tiny              3m12s
development   virtualmachineinstancetype.instancetype.kubevirt.io/csmall                   3m44s

NAMESPACE   NAME                                                                        AGE
default     virtualmachinepreference.instancetype.kubevirt.io/alpine                    3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/centos.7                  3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/centos.7.desktop          3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/centos.8.stream           3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/centos.8.stream.desktop   3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/centos.9.stream           3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/centos.9.stream.desktop   3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/cirros                    3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/fedora                    3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/rhel.7                    3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/rhel.7.desktop            3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/rhel.8                    3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/rhel.8.desktop            3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/rhel.9                    3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/rhel.9.desktop            3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/ubuntu                    3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.10                3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.10.virtio         3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.11                3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.11.virtio         3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.2k12              3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.2k12.virtio       3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.2k16              3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.2k16.virtio       3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.2k19              3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.2k19.virtio       3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.2k22              3m12s
default     virtualmachinepreference.instancetype.kubevirt.io/windows.2k22.virtio       3m12s

@akalenyu
Copy link
Contributor Author

akalenyu commented Mar 29, 2023

/area instancetype

I don't think this is correct, it drops the cluster wide resources from get all -A that we really don't want.

There has to be something else causing these resources to show up in the namespace, I'll try to look deeper now.

Yeah not sure what is the right way to go about this, did not consider get all -A.
but we are definitely outnumbered

$ oc api-resources --namespaced=false -o name | wc -l
143
$ ./script.sh 
virtualmachineclusterinstancetypes.instancetype.kubevirt.io
virtualmachineclusterpreferences.instancetype.kubevirt.io
migrationpolicies.migrations.kubevirt.io

@akalenyu
Copy link
Contributor Author

/area instancetype
I don't think this is correct, it drops the cluster wide resources from get all -A that we really don't want.
There has to be something else causing these resources to show up in the namespace, I'll try to look deeper now.

Yeah not sure what is the right way to go about this, did not consider get all -A. but we are definitely outnumbered

$ oc api-resources --namespaced=false -o name | wc -l
143
$ ./script.sh 
virtualmachineclusterinstancetypes.instancetype.kubevirt.io
virtualmachineclusterpreferences.instancetype.kubevirt.io
migrationpolicies.migrations.kubevirt.io

Sorry we're even more outnumbered, just these 3 kv crds

@lyarwood
Copy link
Member

lyarwood commented Apr 4, 2023

/lgtm

https://github.com/kubernetes/community/blob/master/contributors/devel/sig-cli/kubectl-conventions.md#rules-for-extending-special-resource-alias---all

Finally found some documentation about all (no pun intended) of this and it does highlight that cluster-level resources shouldn't be part of this category.

I've got some supernits around the commit subject and releasenote but I'm okay with this now otherwise. Can you update both to something like the following if you get a chance?

bugfix: Remove all category from VirtualMachineCluster{Instancetype,Preference} CRDs

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 4, 2023
@lyarwood
Copy link
Member

lyarwood commented Apr 4, 2023

Forgot to say that you could likely handle migrationpolicy in a follow up as well.

@mhenriks
Copy link
Member

mhenriks commented Apr 5, 2023

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mhenriks

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 5, 2023
@lyarwood
Copy link
Member

lyarwood commented Apr 5, 2023

/cherrypick release-0.59

@kubevirt-bot
Copy link
Contributor

@lyarwood: once the present PR merges, I will cherry-pick it on top of release-0.59 in a new PR and assign it to you.

In response to this:

/cherrypick release-0.59

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.

@lyarwood
Copy link
Member

lyarwood commented Apr 5, 2023

/retest-required

@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-bot
Copy link
Contributor

kubevirt-bot commented Apr 5, 2023

@akalenyu: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubevirt-fossa 11ababa link false /test pull-kubevirt-fossa

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. I understand the commands that are listed here.

@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-bot
Copy link
Contributor

@lyarwood: new pull request created: #9581

In response to this:

/cherrypick release-0.59

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
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/instancetype dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants