Refactor provider to reliably track changes in manifestgen output #85
Conversation
|
@simongottschlag it would be great if you could have a look at this and comment if you see any major issues. |
phillebaba
force-pushed
the
update/version
branch
3 times, most recently
from
18d3ab2
to
4cd4aee
Compare
| @@ -42,8 +42,7 @@ jobs: | |||
| fi | |||
| - name: Run install terraform | |||
| run: | | |||
| mkdir -p ./examples/install/.terraform/plugins/registry.terraform.io/fluxcd/flux/0.0.1/linux_amd64 | |||
| cp ./bin/flux ./examples/install/.terraform/plugins/registry.terraform.io/fluxcd/flux/0.0.1/linux_amd64/terraform-provider-flux_v0.0.1 | |||
| make install | |||
| cd examples/install | |||
There was a problem hiding this comment.
Suggestion to the end-to-end test:
Add a separate test where you install the latest stable and then run it again with the newly built provider. Should maybe not be a required check, but good to know if something breaks in upgrades.
There was a problem hiding this comment.
Yeap I will try to add this later in its own PR.
| } | ||
| kubectl = { | ||
| source = "gavinbunney/kubectl" | ||
| version = ">= 1.9.1" | ||
| } | ||
| flux = { | ||
| source = "fluxcd/flux" | ||
| version = ">= 0.0.1" | ||
| version = ">= 0.0.10" |
There was a problem hiding this comment.
Would it be possible to change the version during release, making sure it's always the latest stable in the docs?
There was a problem hiding this comment.
Sounds like a good idea but we should add that in another PR.
|
|
||
| resource "kubectl_manifest" "install" { | ||
| for_each = { for v in local.install : lower(join("/", compact([v.data.apiVersion, v.data.kind, lookup(v.data.metadata, "namespace", ""), v.data.metadata.name]))) => v.content } |
There was a problem hiding this comment.
Nice way of handling it! Maybe it's easier to read if it's a parameter of the local and not part of the resource?
There was a problem hiding this comment.
I am trying to avoid too many loops for now.
|
|
||
| resource "kubectl_manifest" "install" { | ||
| for_each = { for v in local.install : lower(join("/", compact([v.data.apiVersion, v.data.kind, lookup(v.data.metadata, "namespace", ""), v.data.metadata.name]))) => v.content } | ||
| depends_on = [kubernetes_namespace.flux_system] | ||
| yaml_body = each.value |
There was a problem hiding this comment.
Will changes to yaml_body trigger a replacement? Is there any risk of the compact statement staying the same but body changing - but not being applied?
There was a problem hiding this comment.
Nope so I have tried upgrading from multiple different versions. The only thing the key does is make sure that the kubectl provider updates resources in place instead of replacing them, when the yaml content changes.
There was a problem hiding this comment.
LGTM
Thanks @phillebaba for explaining to me what all of this does, great improvement 💯
phillebaba
changed the title
Signed-off-by: Philip Laine <philip.laine@gmail.com> Signed-off-by: Philip Laine <philip.laine@xenit.se>
phillebaba
force-pushed
the
update/version
branch
from
4cd4aee
to
c53b7c4
Compare
This change updates to the latest version of fluxv2 dependencies and solve a long standing issue with the examples which could cause issues when upgrading to a new version.
The kubectl provider can only apply single yaml documents per resource, and requires multi doc resources to be split with a special data source. This introduces iterations into the
kubect_manifestresource which introduces state complexity into Terraform. Using count is out of the question as things will have to be replaced (delete, create) as soon as the order of the yaml document changes. Right now in the examples we key with the sha of the content of the individual content of each yaml document, which initially worked fine. That solution proved to be less than optimal when larger changes to the manifests occur, like in the latest version of flux. Instead the new examples creates a resources identifier and uses it has a key in the hopes that Terraform will update the resources in place instead of replacing each resource in Kubernetes.I have created an issue in the kubectl provider that would make this simpler but we need a solution now, up until that can get implemented.
gavinbunney/terraform-provider-kubectl#72
This will also most likely be a problem in the future with terraform-provider-kubernetes-alpha as it does not either support multidoc yaml.