Update required controller-gen to v0.6.1
#135
Conversation
| # | ||
| # Usage: | ||
| # | ||
| # if controller_gen_version_equals "v0.4.0"; then |
There was a problem hiding this comment.
nit: /s/controller_gen_version_equals/k8s_controller_gen_version_equals
| source "$SCRIPTS_DIR/lib/k8s.sh" | ||
|
|
||
| if ! is_installed controller-gen; then | ||
| if ! is_installed controller-gen || ! k8s_controller_gen_version_equals "$CONTROLLER_TOOLS_VERSION"; then |
There was a problem hiding this comment.
Could there be possible collision between which controller-gen binary will be picked up if there is an already installed old version?
IOW, should other version be uninstalled? or better, should the script always install controller-gen in a specific path(which it does) and use complete path to invoke binary to avoid name collision in $PATH ?
There was a problem hiding this comment.
Good point @vijtrip2 , older versions should be uninstalled.
There was a problem hiding this comment.
Many of our ACK contributors also use controller-gen (of different versions) for other projects. I don't want to dictate what version they have on their systems, if they've previously installed something else. From what I've seen, this script overrides the default version of controller-gen by default - but I do think it's a good idea to place it in a logical spot (say in ~/.cache) and re-use that path, rather than putting it into $PATH
There was a problem hiding this comment.
I just looked into this and I don't really want to expand the scope of this particular PR into something like that. I think there are lots of places for errors to creep in. This PR is meant to just increment the version number and clean up some scripts.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RedbackThomson, vijtrip2 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 |
Match changes from aws-controllers-k8s/code-generator#135 for the test containers
Issue #, if available: aws-controllers-k8s/code-generator#135 Description of changes: By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
4 participants
Issue #, if available: aws-controllers-k8s/community#873
Description of changes:
CONTROLLER_TOOLS_VERSIONvariablek8s.shscript (not being used anywhere in this repo)I have tested the new
controller-genagainst thedynamodbcontroller and saw no changes in output CRDs (except for the version number in an annotation). None of the breaking changes called out in the release between0.4.0and0.6.1should have any effect on our generator.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.