Add Kubernetes provider

[radeksimko]

Status (TL;DR)

• kubernetes_limit_range resource + acceptance test
• kubernetes_namespace resource + acceptance test
• kubernetes_persistent_volume resource + acceptance test
• kubernetes_persistent_volume_claim resource + acceptance test
• kubernetes_pod resource + acceptance test
• kubernetes_replication_controller resource + acceptance test
• kubernetes_resource_quota resource + acceptance test
• kubernetes_service resource + acceptance test
• Docs - up to date
• Example
• Decide between KUBERNETES_CLIENT_CERTIFICATE / KUBERNETES_CLIENT_CERTIFICATE_PATH etc.
• Support kubernetes_namespace.finalizers
• Detect default/undefined fields and ignore these when generating diffs
  • kubernetes_limit_range
  • kubernetes_persistent_volume
  • kubernetes_pod
  • kubernetes_replication_controller
  • kubernetes_service

Config ENV vars (w/ and w/out _PATH)

I think the config options should stay as are (accepting keys & crts contents as strings) to provide direct compatibility w/ google_container_engine, yet allowing people to use file(). This is however less convenient for ENV variables since keys & certs are typically multi-line strings and that's a bit tricky to handle in ENV vars. Hence I guess _PATH with some extra logic to read files wins here, but I'll leave this open for a while.

schema.CompareFunc - Why

Creation / deletion of all resources works aok, but the problem is that K8S server returns specs with some extra attributes that we have not defined when sending it to the API originally and these are often not predictable (e.g. node IP address).

I experimented with a few ideas. I decided not to strip attributes inside state (via StateFunc), because I still think that user should be able to reference the real spec from server elsewhere. Hence I created schema.CompareFunc and I'm now trying to come up with a meaningful/universal way of stripping attributes that user has not defined in the given spec, so that we can only compare what has been defined, yet still save the original spec into state.

Mind the test suite

It will also demonstrate the issue with spec as described above.

$ make testacc TEST=./builtin/providers/kubernetes 2>/dev/null

cc @sparkprime @lwander

[apparentlymart]

r.e. passing multi-line cert/key information to providers in the environment, for the Chef provider I started in #3084 I went with the approach of making the property a string that takes the key material itself but making the environment variable be the path to a file to read. As you noted, this requires some extra logic to handle the env var, but not much extra and I think it could generalize well to being reusable in helper/schema like EnvDefaultFunc, if we're gonna use this style across multiple providers.

To me, this felt like the most pragmatic compromise.

[jimmycuadra]

I am surprised and pleased to see this being worked on, but is there precedent for using Terraform to manage software like this? I'd understood Terraform to be for managing cloud resources themselves but not for trying to manage the software that runs on them in anything other than a vary trivial capacity. The idea of having a Terraform resource for something as high level as kubernetes_service seems strange, but maybe I just need to think differently about the goals of the project.

[mitchellh]

@jimmycuadra The theoretical boundary (as designed) for Terraform was always "schedulers" in the most abstract sense. EC2, Azure, SaaS, PaaS, etc. are all schedulers. EC2 schedules "machines", Heroku schedules "apps" and "services", etc.

It makes total sense in this world to include something like Kubernetes, since in practice we find that with any scheduler boundary, you usually have a need for a tool like Terraform to manage resources within that scheduled enviroment (while Terraform manages resources, the schedulers places those resources).

[jimmycuadra]

Ah, I see. I think the key distinction that I missed is that this provider is intended to manage the actual resources a k8s cluster provides using an existing cluster (e.g. GKE, the hosted service from Google Cloud Platform) rather than to stand up the k8s cluster itself.

[lwander]

With regard to the specs the K8S server returns - if I'm understanding schema.CompareFunc correctly, it's passing you the user config spec as well as the state spec with possible spurious parameters. Why not have your custom provided CompareFunc only compare parameters present in the current user config spec? This way the Terraform state stays in sync with the K8S server, but the user config only updates parameters they have defined.

[radeksimko]

Why not have your custom provided CompareFunc only compare parameters present in the current user config spec? This way the Terraform state stays in sync with the K8S server, but the user config only updates parameters they have defined.

That's exactly what I have in mind 😃

Some things will be easy - string/int attributes, some will be a bit more difficult, e.g. pod automatically gets a new default volume with secrets for accessing Google APIs. That will need to be ignored in an array of user-defined volumes.

I'm not sure I'll be able to make this generic enough, so there's just one function to rule them all, but it's ideally the goal.

[lwander]

Ah, I see what you mean!

If you want to do it generically, you could write your own DeepEqual that only compares elements found in the first argument. Unfortunately I can't find anything in the standard library that will do that for you.

[radeksimko]

I was more thinking of using reflect.DeepEqual and generally bunch of other functions from reflect to iterate over that. Ideally I'd want this to work even after Google decides to add more computed attributes in the future - i.e. I don't want to be checking specific attributes per resource, but rather have one CompareFunc that I will be able to use for all K8S resources and eventually even allow others to use in other providers.

[radeksimko]

I know that things like arrays won't be addressable in any generic way (I will have to be resource-specific in filtering arrays), but everything else could be.

[lwander]

I think if you model your compare function after DeepEqual you don't need to check/hardcode specific resources. Instead, check that every element in one argument (an arbitrary user config) exists and matches that in the other (k8s provided state) recursively. This also works for elements of array or splice type, with a little extra logic handling the fact that elements in one array may occur in a different index in the other array. Normally I would rely on the reflect package as well, but I don't see that it provides a fine grained enough way to iterate over arbitrary types.

[lwander]

I don't know what the custom is here since Go doesn't support function overloading, but it might be worthwhile to rewrite the (expand/flatten/normalize)*Spec functions to avoid all the duplicated code.

Also - I see that the K8S API prefers to read the auth config out of a separate json file. Seeing that you're reading most of the K8S specs out of non-Terraform config files, is there a reason why you decided to do it differently for the provider?

What are your thoughts on adding some computed fields to each resource matching the ObjectMeta type? I'm wondering since you mentioned it would be worthwhile to have the Terraform state report details to the user that they don't necessarily have to provide themselves.

I might be wrong, but I don't think you can have namespace be optional for ResourceQuota because the quota is applied per namespace.

[radeksimko]

I don't know what the custom is here since Go doesn't support function overloading, but it might be worthwhile to rewrite the (expand/flatten/normalize)*Spec functions to avoid all the duplicated code.

I will have a look into that, but I'm afraid I'll be balancing between clean (duplicated, but readable) code and long single function that's hard to understand. I'm not sure the abstraction will bring much benefit, but I might be wrong.

Also - I see that the K8S API prefers to read the auth config out of a separate json file. Seeing that you're reading most of the K8S specs out of non-Terraform config files, is there a reason why you decided to do it differently for the provider?

I totally don't mind adding support for JSON file, but I'd like to keep the config options separated, so it works well with google_container_engine just by referencing - fully automated solution.

If it was only supporting JSON file, then workaround would has to be using local-exec to generate a JSON file that can be then referenced by K8S provider, and that I think would be a very messy relationship between resources that I'd expect to work very well together.

What are your thoughts on adding some computed fields to each resource matching the ObjectMeta type? I'm wondering since you mentioned it would be worthwhile to have the Terraform state report details to the user that they don't necessarily have to provide themselves.

There's definitely bunch of attributes that would be worth exposing - e.g. hostname/IP address of a service. I will look into that. This is something I consider easy & quick to solve compared to all other things on the list and other things you mentioned though.

I might be wrong, but I don't think you can have namespace be optional for ResourceQuota because the quota is applied per namespace.

I'm not sure whether it's intention or a bug, but you can set a resource quota for the default namespace - hence it's optional. See related acceptance test for ResourceQuota - it works.

[eirslett]

+1 CompareFunc is really nice. Some providers do strange things with whitespace, so this would let us add a CompareFunc trim(a) == trim(b) 😄

[lwander]

@radeksimko,

I've been thinking it might make more sense to move all the of the attributes in the YAML K8S config files into the Terraform config files. While it will be a bit of work, and it means we have to update the K8S resources as their APIs are improved, we don't get any of the benefits of the Terraform config language as long as the entire K8S resource config is separate from Terraform resource config.

If this sounds good to you, I'll move whichever attributes you need, but we should think about where we want to edit your existing code. Your Terraform fork would probably be easiest.

[radeksimko]

@lwander That's changing the direction completely. When I was submitting ECS, I was first thinking about this too, but then I decided not to break this down.

It would solve some of the mentioned issues, but it also breaks any compatibility we now have with kubectl and potentially other tools that people are already using to manage K8S cluster.

I'm frankly not too inclined to that approach, because any demo that we can now find will likely consist of bunch of YAML/JSON files, that could be just very easily launched via terraform with a little bit of DSL.

Could any core maintainer share opinion on this, please? @mitchellh @phinze

[sparkprime]

If you are launching some stateless web app on Kubernetes and you need it to depend on some database cluster defined also in Terraform with instance resources, can you do that with YAML strings in the schema? If not, I think that would be a major loss. I don't know if a similar case exists for ECS.

Am I understanding correctly that you're saying that it would be hard to take existing k8s configs and port them to Terraform because one would have to convert from YAML/JSON to HCL? There may be other ways to address that, like building a little third party tool to convert.

Another option might be an interpolation syntax that can parse JSON/YAML (from a string) and turn it into structured HCL. That may be useful in other cases too.

Also people (for example me) often use the JSON mode of Terraform, and these large strings I think not mesh well with that approach.

[sparkprime]

MItchell's comments in #3026 are relevant, but I think in this case we are really dealing with structured data that has a given schema, not just "arbitrary JSON" which Terraform doesn't have much ability to process and thus may as well just be a string.

[lwander]

@radeksimko,

Also, at least both the Container (inside the PodSpec)and ServiceSpec contain fields that cannot be changed by updates. Since you don't have either spec marked as ForceNew, a user's configuration changes might mysteriously start failing when Terraform calls your Update method. Marking a Pod or Service as ForceNew isn't a good idea, so with the current approach you're left with generating your own diffs, which is a lot of work that Terraform could be doing for you.

[apparentlymart]

Over in #3124 Radek and I were debating a similar thing for IAM Policy JSON. I'd proposed a compromise where we have the various policy properties take the raw JSON (with some normalization, of course) and then have a separate logical resource to help in producing that JSON. Radek didn't agree with me there of course, so maybe he'll also disagree with me here. 😀

My rationale was that you could in principle generate the JSON using the template_file resource, but for cases like these where there is a well-defined data structure it would be helpful to have a more specialized equivalent of template_file that avoids the need for everyone to reinvent the same template, and makes it easier to include more complex interpolations like globs.

I like the fact that this approach gives you the option of using a pre-existing file if you want, but gives you an easy way to generate a config on the fly if that's more appropriate for your needs. The downside is that it's one more resource to document, and we'd need to explain well in the docs what the difference is between the two so users are not so confused.

[radeksimko]

MItchell's comments in #3026 are relevant, but I think in this case we are really dealing with structured data that has a given schema, not just "arbitrary JSON" which Terraform doesn't have much ability to process and thus may as well just be a string.

Agreed

What I'm also realising is that we might be able to use PATCH more efficiently - we could be updating just part of the Spec, which is one more argument why expand it.

Radek didn't agree with me there of course, so maybe he'll also disagree with me here. 😀

I think after all you convinced me Martin, my concerns about separate IAM document resource were a bit different than here though. Pods, services etc. are still pretty much mapping to the API, that IAM document will be more like terraform_template and potential growth in that direction is my main concern. I guess @sparkprime would suggest using jsonnet to address this. 😃

---

The reason I'm raising this question and need more voices on either side is when a user comes to ask "why did you design it this way and not the other?", we can just point him/her to this thread and/or use the valid arguments you are all mentioning.

If nobody else comes in with a different opinion over the weekend, we can start working on expanding it and I'll be now much happier doing so now than before, since we have enough arguments. 😉

[radeksimko]

Before carrying on with the work on this PR, it would help to have https://github.com/hashicorp/terraform/issues/2275 implemented.

[radeksimko]

As we discussed recently with @lwander & @sparkprime , I resubmitted this under #3453

[ghost]

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.