-
Notifications
You must be signed in to change notification settings - Fork 40.1k
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 test for advanced CRUD for apiextensions #46624
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nikhita Assign the PR to them by writing
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
Hi @nikhita. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with 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. |
hack/make-rules/test-cmd-util.sh
Outdated
|
||
# teardown | ||
kubectl delete customresourcedefinitions/foos.company.com "${kube_flags_with_token[@]}" | ||
kubectl delete customresourcedefinitions/bars.company.com "${kube_flags_with_token[@]}" | ||
} | ||
|
||
run_tpr_tests() { | ||
run_TPR_CRD_tests() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, will fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
* modify the run_crd_tests and run_tpr_tests such that both can be run. * add a spec field for crds Fix wrong rename
dfb8500
to
5a16743
Compare
# Set spec field for CRDs | ||
if [ $1 == CRD ]; then | ||
SPEC=spec. | ||
else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking the else condition should be redundant...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would leave it. makes it clearer. Maybe add to the comment: "For TRPs this is the empty string because TPRs don't have spec and status, but are flat."
kubectl "${kube_flags[@]}" get foos/test -o "jsonpath={.someField}" --allow-missing-template-keys=false | ||
kubectl "${kube_flags[@]}" get foos -o "go-template={{range .items}}{{.someField}}{{end}}" --allow-missing-template-keys=false | ||
kubectl "${kube_flags[@]}" get foos/test -o "go-template={{.someField}}" --allow-missing-template-keys=false | ||
kubectl "${kube_flags[@]}" get foos -o "jsonpath={.items[*].${SPEC}someField}" --allow-missing-template-keys=false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering, aren't these the CRs, not the CRDs? The CRs don't have the spec, do they?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought so too. But they do over here: https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/kube-apiextensions-server/artifacts/customresource-01/noxu.yaml
Looks like this PR is not needed because the status/spec field for CRs does not exist. @deads2k do you think anything needs to be done for checking the kubectl commands here? Or else, I'll close the PR. |
/unassign |
What this PR does / why we need it: Add test for advanced CRUD for apiextensions i.e. check if kubectl commands work right. Added a spec field for CRDs.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): for #45511Special notes for your reviewer:
Release note:
/cc @sttts