Adding K8s remote backend to BYOK #878
Conversation
Codecov Report
@@ Coverage Diff @@
## main #878 +/- ##
==========================================
- Coverage 73.46% 72.93% -0.53%
==========================================
Files 123 123
Lines 8190 8259 +69
==========================================
+ Hits 6017 6024 +7
- Misses 2173 2235 +62
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| kubernetes: | ||
| secret_suffix: "{state_storage}" | ||
| config_path: "{kubeconfig}" |
There was a problem hiding this comment.
How is this change going to affect existing usage of the helm cloud provider?
There was a problem hiding this comment.
- No one is using it
- It will probably lose the current state, trying to recreate everything
Co-authored-by: Patrick Fiedler <[email protected]>
Co-authored-by: Patrick Fiedler <[email protected]>
Co-authored-by: Patrick Fiedler <[email protected]>
There was a problem hiding this comment.
-
There is a 1MB limit for k8s secret, is that enough for us? (how much do we use for a typical service?
-
Don't forget to update the documentation, the part with:
Generate the terraform state files in tfstate - please commit these files after each apply.
https://github.com/run-x/opta/tree/main/examples/byok-eks#deploy-a-service-to-kubernetes-with-opta__
| load_opta_kube_config() | ||
| v1 = CoreV1Api() | ||
| secret_name = f"opta-config-{self.layer.state_storage()}" | ||
| if check_if_secret_exists("default", secret_name): |
There was a problem hiding this comment.
may be we should allow configuring this namespace to something lese than "default" (which would be the default value if not set)
There was a problem hiding this comment.
Not for now I think-- default has the benefit of always being present/available
There was a problem hiding this comment.
ok fair enough, helm also uses to this namespace so we are consistent with that
|
|
||
| def delete_remote_state(self) -> None: | ||
| return None | ||
| v1 = CoreV1Api() | ||
| secret_name = f"tfstate-default-{self.layer.state_storage()}" |
There was a problem hiding this comment.
no need to have default here since it's a namespace scoped resource
There was a problem hiding this comment.
That's how terraform names the secret-- we got no choice :(
|
@RemyDeWolf Don't forget to update the documentation, |
opta/core/helm_cloud_client.py
Outdated
| except Exception: | ||
| with open(default_kube_config_filename) as f: | ||
| default_kube_config = yaml.load(f) | ||
| except YAMLError: |
There was a problem hiding this comment.
| except YAMLError: | |
| except yaml.YAMLError: |
(as opposed to from ruamel.yaml import YAMLError above). The yaml util library should be used instead of importing ruamel yaml directly, to discourage unsafe usage. That library reexports YAMLError.
Description
Now byo k8s cluster will store its states using k8s secrets via the official k8s backend
Safety checklist
How has this change been tested, beside unit tests?
manually