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

kubernetes: Add provider + namespace resource #12372

Merged
merged 2 commits into from
Mar 16, 2017
Merged

kubernetes: Add provider + namespace resource #12372

merged 2 commits into from
Mar 16, 2017

Conversation

radeksimko
Copy link
Member

@radeksimko radeksimko commented Mar 2, 2017

I intentionally made this PR as minimal as possible (provider + 1 resource + relevant tests + docs) to make it easier for review. Other resources and potentially data sources may follow.

Test plan

make testacc TEST=./builtin/providers/kubernetes
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/02 11:54:13 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/kubernetes -v  -timeout 120m
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccKubernetesNamespace_basic
--- PASS: TestAccKubernetesNamespace_basic (6.66s)
=== RUN   TestAccKubernetesNamespace_importBasic
--- PASS: TestAccKubernetesNamespace_importBasic (5.78s)
=== RUN   TestAccKubernetesNamespace_generatedName
--- PASS: TestAccKubernetesNamespace_generatedName (5.63s)
=== RUN   TestAccKubernetesNamespace_importGeneratedName
--- PASS: TestAccKubernetesNamespace_importGeneratedName (6.14s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/kubernetes	24.288s

History Lesson

There have been a couple of PRs with K8S provider in the past:

which were closed (unmerged) for a number of reasons, TL;DR:

Both mentioned things are being addressed in https://github.com/radeksimko/terraform-gen, although we might not be able to address these as well as I'd hope, mainly because the source SDK structs aren't concrete enough for our schema (e.g. validation, list vs set, hashing mechanisms etc.). But at least folks can get going quickly from zero, generate skeleton of schema + docs and then modify and do the rest manually.

@paddycarver
Copy link
Contributor

Spent a couple days reviewing this in depth and reading up on the history, and it gets a 👍 from me. :)

@radeksimko
Copy link
Member Author

Thanks Paddy, I appreciate the extra time invested into the review!

@radeksimko radeksimko merged commit 4448e45 into master Mar 16, 2017
@radeksimko radeksimko deleted the f-kubernetes branch March 16, 2017 07:18
@paddycarver
Copy link
Contributor

My pleasure! Sorry for the delay, I got confused and wanted to understand done background before signing off on it.

"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/terraform"
"github.com/mitchellh/go-homedir"
kubernetes "k8s.io/kubernetes/pkg/client/clientset_generated/release_1_5"
Copy link

Choose a reason for hiding this comment

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

Shouldn't client-go be used instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I really wanted to use that library when starting the work on K8S provider, but it was practically impossible to use it as a vendored dependency due to some upstream issues/conflicts which I described in #12230 (comment)

We might switch to that library at some point in the future to reduce the number of vendored LOC and because that's the library that should be used for that purpose, but the main goal is to get initial coverage of all stable (v1) k8s resources for now so re-vendoring is not currently on my radar.

I hope that's understandable 😃

Copy link

Choose a reason for hiding this comment

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

Totally understandable. Keep up!

@ghost
Copy link

ghost commented Apr 14, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants