-
Notifications
You must be signed in to change notification settings - Fork 49
Constant diff in lokoctl cluster apply
#24
Comments
@johananl perhaps this issue should be moved to https://github.com/kinvolk/lokomotive. TL;DR the issue is how |
Can't we use |
No, because what we need is copying directories, which is not-trivial with Terraform right now, and |
Interesting, it seems running |
It seems we do. Actually, now Terraform shows the plan as well. Maybe re-initializing Terraform breaks it somehow then 🤔 |
FYI, following patch prevents continuous diff, but if assets directory gets removed, it still occurs: diff --git a/pkg/util/walkers/copying.go b/pkg/util/walkers/copying.go
index dd82612b..31cfcabb 100644
--- a/pkg/util/walkers/copying.go
+++ b/pkg/util/walkers/copying.go
@@ -16,9 +16,11 @@ package walkers
import (
"fmt"
+ "reflect"
"io"
"os"
"path/filepath"
+ "io/ioutil"
"github.com/pkg/errors"
@@ -44,6 +46,15 @@ func CopyingWalker(path string, newDirPerms os.FileMode) assets.WalkFunc {
// writeFile writes data from given io.Reader to the file and makes sure, that
// this is the only content stored in the file.
func writeFile(p string, r io.Reader) error {
+ newContent, err := fileNeedsUpdate(p, r)
+ if err != nil {
+ return fmt.Errorf("failed checking if file needs to be updated: %w", err)
+ }
+
+ if newContent == nil {
+ return nil
+ }
+
// TODO: If we start packing binaries, make sure they have executable bit set.
f, err := os.OpenFile(p, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644)
if err != nil {
@@ -51,9 +62,36 @@ func writeFile(p string, r io.Reader) error {
}
defer f.Close()
- if _, err := io.Copy(f, r); err != nil {
+ if _, err := f.Write(newContent); err != nil {
return fmt.Errorf("failed writing to file %s: %w", p, err)
}
return nil
}
+
+func fileNeedsUpdate(p string, r io.Reader) ([]byte, error) {
+ fc, err := ioutil.ReadAll(r)
+ if err != nil {
+ return nil, fmt.Errorf("failed reading file content from assets: %w", err)
+ }
+
+ _, err = os.Stat(p)
+ if os.IsNotExist(err) {
+ return fc, nil
+ }
+
+ if err != nil {
+ return nil, fmt.Errorf("failed to stat file %s: %w", p, err)
+ }
+
+ ofc, err := ioutil.ReadFile(p)
+ if err != nil {
+ return nil, fmt.Errorf("failed reading file %s for content comparison: %w", p, err)
+ }
+
+ if reflect.DeepEqual(ofc, fc) {
+ return nil, nil
+ }
+
+ return fc, nil
+} |
So it seems this constant diff is being created because modification time changes on source files. This can be reproduced on a single example: provider "template" {
version = "2.1.2"
}
resource "template_dir" "test" {
source_dir = "./src"
destination_dir = "./dest"
} Now the question is, what to do with that, as I think this behavior is technically correct in my opinion. Option 1Add some parameter to Considering, that upstream Terraform providers are horribly maintained and inability of Terraform to easily fetch custom providers, I'd rather avoid this option. Option 2Get rid of Option 3Avoid copying directories completely somehow. |
Should we at least document it, so users don't find it confusing? |
lokoctl cluster install
lokoctl cluster apply
I wouldn't document a bug in the product docs :-/ I suggest we just fix this ASAP. |
From terraform docs (https://www.terraform.io/docs/providers/template/r/dir.html)
|
@ipochi I don't think this is it. We get this diff even when re-running |
NOTE: this will change with upcoming Terraform 0.13, as it will allow to pull provider from 3rd party repositories, so we can just have our own repository then, with patched version of provider. Definitely not the nicest approach, but one of the easiest in my opinion. |
I'm currently evaluating an approach which gets rid of I am also considering looking into fetching charts from a Helm repo instead of copying them over SSH. If it's not too crazy or out of scope for this issue, doing so could move us closer to the idea of self-contained release packages decoupled from |
This seems to work nicely and it fixes the constant diff problem. I still need to address two cases which require passing the platform config all the way down to the module which generates files from memory to disk:
|
Before this change, control plane charts are generated to disk as part of the `bootkube` Terraform module. Then, when Terraform runs, the charts are copied to `$assets/cluster-assets` using `template_dir` Terraform resources. This additional copy operation causes Terraform diff bugs and is also technically unnecessary since the chart files aren't being templated by Terraform and are rather copied as-is. As a byproduct, this change decouples the charts from Terraform code, which moves us in the direction of decoupling Lokomotive releases from the `lokoctl` binary. It could make sense to handle charts belonging to Lokomotive components in a similar way. Fixes #24.
Fixes #24. - Generate control plane charts into cluster assets dir. Before this change, control plane charts are generated to disk as part of generating the `bootkube` Terraform module. Then, when Terraform runs, the charts are copied to `$assets/cluster-assets` using `template_dir` Terraform resources. This additional copy operation causes Terraform diff bugs and is also technically unnecessary since the chart files aren't being templated by Terraform and are rather copied as-is. - Re-organize charts. Manage all charts under `charts/` with separate subdirectories for control plane and component charts. Put component chart files in `charts/components/<component_name>` (without an extra `/manifests` parent dir). - Add support for platform-specific charts. There are cases where a specific control plane chart needs to be deployed only on specific platforms (e.g. host protection is used only on Packet). To handle this, we define a set of "common" charts which are always deployed and allow platform implementations to add additional charts using arbitrary Go code. - Add support for optional charts. Since we now use Go code to determine which charts get deployed for a platform, we can make arbitrary checks (e.g. evaluating a config knob) before deciding to deploy a chart. We already use this approach to deploy the self-hosted kubelet only when enabled in the cluster config. - Define a generic `Extract()` function under `assets` and use it instead of functions such as `PrepareLokomotiveTerraformModuleAt()` which are too "specialized". - Delete `install.go`. Now that we use the generic `Extract()` function to extract assets we no longer need the specialized extraction functions in this file. After removing the specialized functions, the file becomes empty and therefore should be removed.
Fixes #24. - Generate control plane charts into cluster assets dir. Before this change, control plane charts are generated to disk as part of generating the `bootkube` Terraform module. Then, when Terraform runs, the charts are copied to `$assets/cluster-assets` using `template_dir` Terraform resources. This additional copy operation causes Terraform diff bugs and is also technically unnecessary since the chart files aren't being templated by Terraform and are rather copied as-is. - Re-organize charts. Manage all charts under `charts/` with separate subdirectories for control plane and component charts. Put component chart files in `charts/components/<component_name>` (without an extra `/manifests` parent dir). - Add support for platform-specific charts. There are cases where a specific control plane chart needs to be deployed only on specific platforms (e.g. host protection is used only on Packet). To handle this, we define a set of "common" charts which are always deployed and allow platform implementations to add additional charts using arbitrary Go code. - Add support for optional charts. Since we now use Go code to determine which charts get deployed for a platform, we can make arbitrary checks (e.g. evaluating a config knob) before deciding to deploy a chart. We already use this approach to deploy the self-hosted kubelet only when enabled in the cluster config. - Define a generic `Extract()` function under `assets` and use it instead of functions such as `PrepareLokomotiveTerraformModuleAt()` which are too "specialized". - Delete `install.go`. Now that we use the generic `Extract()` function to extract assets we no longer need the specialized extraction functions in this file. After removing the specialized functions, the file becomes empty and therefore should be removed.
Fixes #24. - Generate control plane charts into cluster assets dir. Before this change, control plane charts are generated to disk as part of generating the `bootkube` Terraform module. Then, when Terraform runs, the charts are copied to `$assets/cluster-assets` using `template_dir` Terraform resources. This additional copy operation causes Terraform diff bugs and is also technically unnecessary since the chart files aren't being templated by Terraform and are rather copied as-is. - Re-organize charts. Manage all charts under `charts/` with separate subdirectories for control plane and component charts. Put component chart files in `charts/components/<component_name>` (without an extra `/manifests` parent dir). - Add support for platform-specific charts. There are cases where a specific control plane chart needs to be deployed only on specific platforms (e.g. host protection is used only on Packet). To handle this, we define a set of "common" charts which are always deployed and allow platform implementations to add additional charts using arbitrary Go code. - Add support for optional charts. Since we now use Go code to determine which charts get deployed for a platform, we can make arbitrary checks (e.g. evaluating a config knob) before deciding to deploy a chart. We already use this approach to deploy the self-hosted kubelet only when enabled in the cluster config. - Define a generic `Extract()` function under `assets` and use it instead of functions such as `PrepareLokomotiveTerraformModuleAt()` which are too "specialized". - Delete `install.go`. Now that we use the generic `Extract()` function to extract assets we no longer need the specialized extraction functions in this file. After removing the specialized functions, the file becomes empty and therefore should be removed.
Fixes #24. - Generate control plane charts into cluster assets dir. Before this change, control plane charts are generated to disk as part of generating the `bootkube` Terraform module. Then, when Terraform runs, the charts are copied to `$assets/cluster-assets` using `template_dir` Terraform resources. This additional copy operation causes Terraform diff bugs and is also technically unnecessary since the chart files aren't being templated by Terraform and are rather copied as-is. - Re-organize charts. Manage all charts under `charts/` with separate subdirectories for control plane and component charts. Put component chart files in `charts/components/<component_name>` (without an extra `/manifests` parent dir). - Add support for platform-specific charts. There are cases where a specific control plane chart needs to be deployed only on specific platforms (e.g. host protection is used only on Packet). To handle this, we define a set of "common" charts which are always deployed and allow platform implementations to add additional charts using arbitrary Go code. - Add support for optional charts. Since we now use Go code to determine which charts get deployed for a platform, we can make arbitrary checks (e.g. evaluating a config knob) before deciding to deploy a chart. We already use this approach to deploy the self-hosted kubelet only when enabled in the cluster config. - Define a generic `Extract()` function under `assets` and use it instead of functions such as `PrepareLokomotiveTerraformModuleAt()` which are too "specialized". - Delete `install.go`. Now that we use the generic `Extract()` function to extract assets we no longer need the specialized extraction functions in this file. After removing the specialized functions, the file becomes empty and therefore should be removed.
#716 is a blocker following feedback on the PR. |
Hm, as |
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
- Move all charts under `assets/charts` (both component charts and control plane charts). Managing all chart assets under a single path makes it easy to organize them and handle chart extraction in a generic way in the code. - Make component paths consistent. Some components had the chart files under `manifests/` whereas others had them in the component's root directory. - Ensure a component's dir name matches the component's name. This allows finding component chart files by *convention*, i.e. using a path such as `assets/charts/components/<name>`. - Extract control plane charts from Go code instead of using the `template_dir` Terraform resource. Fixes #24.
When re-running
lokoctl cluster install
on a cluster which was just deployed and should therefore be aligned with the desired configuration, we see a constant diff:Confirming the change results in a successful
terraform apply
which doesn't seem to have any negative effects, however the diff probably shouldn't be there.The text was updated successfully, but these errors were encountered: