-
Notifications
You must be signed in to change notification settings - Fork 4
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
Update Terrajet #7
Conversation
Signed-off-by: Sverre Boschman <[email protected]>
…-template Signed-off-by: Sverre Boschman <[email protected]>
Signed-off-by: Sverre Boschman <[email protected]>
Signed-off-by: Sverre Boschman <[email protected]>
Signed-off-by: Sverre Boschman <[email protected]>
Hopefully you can find some time to have a look at this @ytsarev ? |
@sboschman sure, did you make any e2e tests after regeneration? do basic examples work? |
Signed-off-by: Sverre Boschman <[email protected]>
Does this project have any e2e tests, did not see them? As you need a Rancher instance to connect with, making e2e tests for a crossplane providers is probably not a trivial task? See #8 en #9, (gke) cluster and namespace resources are working, so I would assume the ProviderConfig part, api client and general provider functions are working. |
@sboschman I meant if you managed to try to instantiate the example resources against real rancher instance in general, no need to extend pipeline with it at the moment |
Catalog example works if you take into account that Terrajet has no knowledge of 'default' values as described in the upstream terraform provider docs. So the scope = global has to be explicitly set with crossplane:
Status:
Rancher api browsing to check creation:
ProviderConfig works if you take into account #4 StoreConfig uses Vault, we don't use Vault, so I don't have access to any Vault stuff to test, example is from the template repo so... |
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.
@sboschman that looks great, thank you so much for this contribution!
Description of your changes
This PR upgrades to the latest Terrajet, based on the provider-jet-template repository, including a bump to go 1.18, and bumps terraform-provider-rancher2 to 1.24.0.
In its current state I am unable to get this provider generating anything to work on fixing some other open issues. I end up in panics, golang dependency hell, compile errors or partly generated resources. This is most likely my lack of golang skills in combination with my environment (golang 1.18). So I opted to start with upgrading the provider to the latest Terrajet build using the template repo.
To, hopefully, make the review less painful the process is split in a few commits.
Commit 1: Update provider-jet-template
This is the initial repo commit if you start a provider from scratch using the provider-jet-template repo and following the first steps from the docu to customise the template. Not the most relevant commit imo, as the next commit is the merge and shows the diff vs the provider-jet-rancher repo.
Commit 2: Merge remote-tracking branch
This is the commit merging the commit above into the existing provider-jet-rancher repo. The 'build' directory shows up as a lot of removed files. Reason is switching it to a git submodule, as per the template repo.
Commit 3: Bump go
Bumping go to 1.18 and pinning golang.org/x/tools to v0.1.11 (fixes a panic, see crossplane/terrajet#271) and running a go mod tidy afterwards.
Commit 4: Bump terraform-provider-rancher2
Bump to v1.24.0
Commit 5: Regenerate provider schema
Running
make generate
to recreate everything. All changes in this commit are the result of runningmake generate
.I have:
make reviewable test
to ensure this PR is ready for review.(yes, golangci-lint throws a 'unknown iexport format version 2' exception)
How has this code been tested
make clean; make generate
runs without reporting any errorsmake run
starts the controller without errors (after importing the CRDs to k8s)make build
throws an exception, seems to be a known issue (bug in file cluster/images/provider-jet-github-controller/Dockerfile provider-jet-template#25), leaving a possible fix for a followup PR as not to clutter even more stuff into this PR.