⚠ Add testenv support for CRDv1/CRDv1beta1 + update to k8s 1.16.4 #752
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/test pull-controller-runtime-test |
|
The failure seems like a race condition, it doesn't happen locally. I can take a look after getting some feedback on the current approach |
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
|
/retest |
|
@vincepri looks reasonable, I will test locally as well, but kicking the flake for now. |
|
/assign gerred |
|
@gerred @DirectXMan12 Thoughts on how to proceed here? |
k8s-ci-robot
added
the
needs-rebase
vincepri
force-pushed
the
support-testenv-crdv1
branch
from
c45e6d8
to
cd35807
Compare
k8s-ci-robot
removed
the
needs-rebase
|
looks like it's passing. Will defer to @gerred for the LGTM. Let me know when that's in place |
|
In transit at the moment but I will review again and lgtm here (or have any last feedback) in an hour or two!
… On Jan 14, 2020, at 10:02 PM, Solly Ross ***@***.***> wrote:
looks like it's passing. Will defer to @gerred for the LGTM. Let me know when that's in place
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
/lgtm
|
k8s-ci-robot
added
the
lgtm
|
@DirectXMan12 clear to approve, tyvm!
|
| @@ -19,7 +19,7 @@ set -e | |||
| hack_dir=$(dirname ${BASH_SOURCE}) | |||
| source ${hack_dir}/common.sh | |||
|
|
|||
| k8s_version=1.14.1 | |||
| k8s_version=1.16.4 | |||
There was a problem hiding this comment.
Can we mention this version upgrade in the PR message itself. Is CRD with version v1 supported from 1.16.4?
| gvs := []schema.GroupVersion{} | ||
| if crd.Spec.Version != "" { | ||
| gvs = append(gvs, schema.GroupVersion{Group: crd.Spec.Group, Version: crd.Spec.Version}) | ||
| crdGroup, _, err := unstructured.NestedString(crd.Object, "spec", "group") |
There was a problem hiding this comment.
Will it be good to check the found option & return err for both spec.group not found as well err cases?
There was a problem hiding this comment.
I'm ok either way, although it's a change from the previous behavior
|
@AmitKumarDas: changing LGTM is restricted to collaborators In response to this:
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. |
vincepri
changed the title
Signed-off-by: Vince Prignano <vincepri@vmware.com>
vincepri
force-pushed
the
support-testenv-crdv1
branch
from
cd35807
to
eb9e87f
Compare
k8s-ci-robot
removed
the
lgtm
|
@DirectXMan12 @gerred Any updates on the status of this PR? |
|
@vincepri I can't approve as a reviewer. @DirectXMan12 @droot @pwittrock are we OK approving here? I don't mind also becoming a CR approver to help here more. :) |
|
/approve as per the reviewers' LGTMs This has the nice effect of getting CRD out of our public API surface. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DirectXMan12, vincepri 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 |
k8s-ci-robot
added
the
approved
|
Thanks @DirectXMan12 ! @gerred need another lgtm if you're around :) |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
Doc Clarification: Using an External Type
Signed-off-by: Vince Prignano vincepri@vmware.com
This PR adds support for testenv to load CRDv1 and CRDv1beta1 by using
runtime.Objectand controller-runtime unstructured client.