✨ [WIP] use local_mode with cspo by kranurag7 · Pull Request #78 · SovereignCloudStack/cluster-stack-provider-openstack

kranurag7 · 2024-02-06T09:13:13Z

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

squash commits
include documentation
add unit tests

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

janiskemper · 2024-02-06T10:03:25Z

@kranurag7 plz add the documentation for this mode as well, we have it already in the CSO I believe!

Co-authored-by: Roman Hros <roman.hros@dnation.cloud> Signed-off-by: kranurag7 <81210977+kranurag7@users.noreply.github.com>

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

janiskemper · 2024-02-06T11:58:26Z

 If you have any trouble finding the right command, then you can use `make help` to get a list of all available make targets.
+
+## Toggle between local_mode and remote mode 
+We can retrieve cluster-stacks in two modes. One way is to let the controller fetch it from GitHub which is remote mode and other is we mount the cluster-stacks inside the container and start the controller with appropriate flag. 


you don't have to specify the flag, remember? This is the feature of CSO, not of CSPO. Can you adapt this to not confuse people?

But IMO we should also introduce this feature in the CSPO, right? Because when you are using local_mode you can hit GitHub API rate limit exceeded error. In CSO, you resolved it by changing these lines, am i correct?

yes but github will never be called anyway if the cluster stack release is available in the container already. So the flag is mainly to prevent the situation that the user uses a certain release, expects this version to be available local, and in reality accidentally downloads it from a remote repository.
By using a fake github client this can be prevented.

It's a small thing but if you'd like to implement it, definitely go ahead! And maybe it's correct that you would call Github unnecessarily in case a release is not available locally and it always tries to find it on Github.

So I think we will also implement this feature for CSPO because here(this is the same part of code, which is also in the CSO) it is called every time you spawn tilt and apply/create openstackclusterstackrelease. So if you do make tilt-up and then destroy the bootstrap cluster and you make tilt-up again in a short period of time then the error can come up cause of this, when you do not provide GIT_ACCESS_TOKEN_B64 variable.

@janiskemper
Why do you create a gh client before checking if the assets should be downloaded? Why not create only, in the download condition?

Feel free to open another issue @chess-knight

Ok issue is here #80

@michal-gubricky then we probably do not need fake gh client in the local mode

Ok issue is here #80

@michal-gubricky then we probably do not need fake gh client in the local mode

Yes, I think so.

local mode introduces a fake github for cases where the cluster stack is not available in the container.. otherwise you won't need the local mode at all

because the "normal" mode already does this.. Downloading only when necessary

janiskemper · 2024-02-06T11:59:39Z

+> [!NOTE]
+> In this mode you need to have cluster-stacks present locally. 
+
+Downloading cluster-stacks can be achieved by many ways but below is a simple way to download it quickly. 


this won't work as this cluster stack is not adapted for csmctl... Can you adapt the docs so that they contain a local workflow with csmctl?

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

chess-knight · 2024-02-07T11:40:40Z

I found IMO huge problem here. The .release directory is synced to the cspo operator but not to the cso operator and then cso tries to download a non-existent release from remote.

Yes, very good catch.
I think we will have to move the cso deployment in the tree and use multistage build to mount the release assets inside cso.

@chess-knight I've pushed a fix for this moving the cso deployment in the repo itself rather than fetching it directly from GitHub.
Please see and test this commit here 807ffa7
Appreciate your review on this one.

Thank you @kranurag7 for that commit, I reviewed it by reading only and wrote down some questions for you. Now I plan to test it and maybe write some other comments, will see

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

michal-gubricky · 2024-02-09T09:43:06Z

+            "cso-manager-rolebinding:clusterrolebinding",
+            "cso-serving-cert:certificate",
+            "cso-selfsigned-issuer:issuer",
+            "cso-validating-webhook-configuration:validatingwebhookconfiguration",


This line should probably be commented out, otherwise I got an error: Internal error occurred: failed calling webhook "validation.clusterstack.clusterstack.x-k8s.io"

CSO has commented it here https://github.com/SovereignCloudStack/cluster-stack-operator/blob/main/Tiltfile#L210 but probably here it somehow should work because I do see cso-validating-webhook-configuration in the cso-components.yaml.
But maybe we won't need that according to my other comments, e.g. when cso-components.yaml will be removed from local and downloaded from CSO release instead

Actually, it seems like there is no webhook server running in cso manager, am I right?

It doesn't work even when this line is commented.

I have to comment out also ValidatingWebhookConfiguration from cso-components.yaml but then GIT_ env vars are missing:

$ k get clusterstack -n cluster NAME PROVIDER CLUSTERSTACK K8S CHANNEL AUTOSUBSCRIBE USABLE LATEST AGE REASON MESSAGE clusterstack openstack scs 1.27 stable false 99s GitTokenOrEnvVariableNotSet failed to create git config: environment variable GIT_PROVIDER is not set

It is because secret cso-cluster-stack-variables, as I pointed out in #78 (comment), is missing and also there is no env from secret mapping in the cso deployment.

Actually, it seems like there is no webhook server running in cso manager, am I right?

Yes, there is not webhook server running in cso as of now but it's a WIP from my side and I mixed it up. I just use kustomize build against my development branch and got the cso-components.yaml file. I'll sync/clean the cso-components file with what we have in GitHub release.

chess-knight · 2024-02-09T09:48:46Z

How and why does this file differ from v0.1.0-alpha.2 release cso-infrastructure-components.yaml?

This came out directly from development branch and doesn't reflect what we have released in CSO. I'll sync it with GitHub release as per the comment above.

chess-knight · 2024-02-09T09:49:55Z

-    cmd = "curl -sSL {} | {} | kubectl apply -f -".format(cso_uri, envsubst_cmd)
-    local(cmd, quiet = True)
+    # yaml = str(kustomizesub("./hack/observability")) # build an observable kind deployment by default
+    yaml_cso = './cso-components.yaml'


Why do we want to have local cso yaml files and not download them from release?

I am thinking about something like this:

version = settings.get("cso_version") cso_uri = "https://github.com/sovereignCloudStack/cluster-stack-operator/releases/download/{}/cso-infrastructure-components.yaml".format(version) cmd = "curl -sSL {} | {}".format(cso_uri, envsubst_cmd) yaml_cso = local(cmd, quiet = True)

chess-knight · 2024-02-09T09:51:01Z

+            target = "tilt",
+            entrypoint = entrypoint,
+            live_update = [
+                sync("cso-components.yaml", "/cso-components.yaml"),


Why do we need sync this file into the docker container?

I had to sync something because this was a required parameter for using docker_build_with_restart
This was the main reason behind this sync.

chess-knight · 2024-02-09T09:53:53Z

+## This should have the same versions as the Dockerfile
+if settings.get("local_mode"):
+    tilt_dockerfile_header_cso = """
+    FROM ghcr.io/sovereigncloudstack/cso:v0.1.0-alpha.2 as builder


Can the settings.get("cso_version") be used here?

chess-knight · 2024-02-09T09:56:36Z

+            ],
+            ignore = ["templates"],
+        )
+    else:


It is possible to docker build for the local mode and apply remote yamls for non-local mode?

Yes, it's possible for sure.
That's how I implemented it first.
I define two functions deploy_cso for non-local mode and used what we have earlier and another function deploy_local_cso for local mode.
The if condition then goes in the last block.

if settings.get("local_mode"): deploy_local_cso() else: deploy_cso()

But I don't have a strong opinion about this approach either. If you think it should be done using two different function then please let me know.

chess-knight · 2024-02-09T09:59:03Z

+    # build into the container.
+    if settings.get("local_mode"):
+        docker_build_with_restart(
+            ref = "ghcr.io/sovereigncloudstack/cso-staging",


So do we use cso-staging or cso:v0.1.0-alpha.2?

Since we plan on using cso-components.yaml directly from GitHub. I think using ghcr.io/sovereigncloudstack/cso:v0.1.0-alpha.2 this sounds better. I'll update accordingly.

chess-knight · 2024-02-09T10:05:24Z

+            "cso-manager-rolebinding:clusterrolebinding",
+            "cso-serving-cert:certificate",
+            "cso-selfsigned-issuer:issuer",
+            "cso-validating-webhook-configuration:validatingwebhookconfiguration",


CSO has commented it here https://github.com/SovereignCloudStack/cluster-stack-operator/blob/main/Tiltfile#L210 but probably here it somehow should work because I do see cso-validating-webhook-configuration in the cso-components.yaml.
But maybe we won't need that according to my other comments, e.g. when cso-components.yaml will be removed from local and downloaded from CSO release instead

Actually, it seems like there is no webhook server running in cso manager, am I right?

chess-knight · 2024-02-09T10:09:41Z

+            "cso-leader-election-rolebinding:rolebinding",
+            "cso-manager-rolebinding:clusterrolebinding",
+            "cso-serving-cert:certificate",
+            "cso-selfsigned-issuer:issuer",


Will the remote mode work when there is no cso-cluster-stack-variables:secret here also I do not see it in cso-components.yaml

I think this is not required and I'll remove it in the next commit.

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

michal-gubricky · 2024-02-12T09:44:50Z

    cso_uri = "https://github.com/sovereignCloudStack/cluster-stack-operator/releases/download/{}/cso-infrastructure-components.yaml".format(version)
    cmd = "curl -sSL {} | {} | kubectl apply -f -".format(cso_uri, envsubst_cmd)
-    local(cmd, quiet = True)
+    yaml_cso = str(local(cmd, quiet = True))


Suggested change

yaml_cso = str(local(cmd, quiet = True))

local(cmd, quiet = True)

I had to re-trigger the cspo-template one. It's not green when it's starting. Re-triggering it works but it would be great to have it green in the starting only.

Details

Yes, we know that and I think it is in the readme too. I don't have much experience with Tilt, so if you know how to make the tilt wait for the CRDs to be applied first and then apply cspo-template.yaml with filled env variables, that would be great.

@kranurag7 please see #84

chess-knight

I am still unsuccessful with syncing .release into the CSO

Co-authored-by: Michal Gubricky <michal.gubricky@dnation.cloud> Signed-off-by: kranurag7 <81210977+kranurag7@users.noreply.github.com>

kranurag7 · 2024-02-12T11:35:28Z

I tested both local and remote mode and I think it's working for me.

remote mode

Details

local mode

Details

chess-knight · 2024-02-12T11:39:46Z

I tested both local and remote mode and I think it's working for me.

Are you sure, that the local mode for CSO works ? .. according to the img, I think it is pulled from the remote. In the local mode, CSPO and CSO should have the same content in /tmp/downloads/cluster-stacks synced with .release

kranurag7 · 2024-02-12T11:41:03Z

Yes, you're right. I think the cso one is still downloading from GitHub because node-images.yaml is not there. I'm looking into it.

kranurag7 · 2024-02-12T15:22:16Z

I think we will have to use our own image for cso as well. We build the cso image and push it to the local registry. If we deploy it directly via the config present in release asset then I don't think we will be able to sync the .release into the directory.

Details

def deploy_cso():
   version = settings.get("cso_version")
   cso_uri = "https://github.com/sovereignCloudStack/cluster-stack-operator/releases/download/{}/cso-infrastructure-components.yaml".format(version)
   cmd = "curl -sSL -o cso-components.yaml {}".format(cso_uri)
   yaml_cso = str('./cso-components.yaml')
   local_resource(
       name = "cso-components",
       cmd = ["sh", "-ec", sed_cmd, yaml_cso, "|", envsubst_cmd],
       labels = ["CSO"],
   )
   entrypoint = ["/manager"]
   extra_args = settings.get("extra_args")
   if extra_args:
       entrypoint.extend(extra_args)
   if settings.get("local_mode"):
       docker_build_with_restart(
           ref = "ghcr.io/sovereigncloudstack/cso",
           context = ".",
           dockerfile_contents = tilt_dockerfile_header_cso,
           target = "tilt",
           entrypoint = entrypoint,
           live_update = [
               sync(".release", "/tmp/downloads/cluster-stacks"),
           ],
           ignore = ["templates"],
       )
   else:
       docker_build_with_restart(
           ref = "ghcr.io/sovereigncloudstack/cso",
           context = ".",
           dockerfile_contents = tilt_dockerfile_header_cso,
           target = "tilt",
           entrypoint = entrypoint,
           live_update = [
               sync("cso-components.yaml", "/cso-components.yaml"),
           ],
           ignore = ["templates"],
       )
   k8s_yaml(yaml_cso)
   k8s_resource(workload = "cso-controller-manager", labels = ["CSO"])
   k8s_resource(
       objects = [
           "cso-system:namespace",
           "clusteraddons.clusterstack.x-k8s.io:customresourcedefinition",
           "clusterstackreleases.clusterstack.x-k8s.io:customresourcedefinition",
           "clusterstacks.clusterstack.x-k8s.io:customresourcedefinition",
           "cso-controller-manager:serviceaccount",
           "cso-cluster-stack-variables:secret",
           "cso-leader-election-role:role",
           "cso-manager-role:clusterrole",
           "cso-leader-election-rolebinding:rolebinding",
           "cso-manager-rolebinding:clusterrolebinding",
           "cso-serving-cert:certificate",
           "cso-selfsigned-issuer:issuer",
       ],
       new_name = "cso-misc",
       labels = ["CSO"],
   )

I used the deploy_cso function defined above, but this time I am running into issue where tilt is not able to decode the secret. Below is the logs from tilt UI.
Error in k8s_yaml: decoding Secret "cso-cluster-stack-variables": illegal base64 data at input byte 0

I understand the error but it's not expected. The same config is working for cspo but in case of cso it's giving me an error. kustomize build config/default gives us similar secret for cspo pod as well which tilt is able to decode and apply using k8s_yaml.

Doing it manually works.

$ cat cso-components.yaml | sed 's/:=\"\"//g' | envsubst | kubectl apply -f -
namespace/cso-system created
customresourcedefinition.apiextensions.k8s.io/clusteraddons.clusterstack.x-k8s.io created
customresourcedefinition.apiextensions.k8s.io/clusterstackreleases.clusterstack.x-k8s.io created
customresourcedefinition.apiextensions.k8s.io/clusterstacks.clusterstack.x-k8s.io created
serviceaccount/cso-controller-manager created
role.rbac.authorization.k8s.io/cso-leader-election-role created
clusterrole.rbac.authorization.k8s.io/cso-manager-role created
rolebinding.rbac.authorization.k8s.io/cso-leader-election-rolebinding created
clusterrolebinding.rbac.authorization.k8s.io/cso-manager-rolebinding created
secret/cso-cluster-stack-variables created
deployment.apps/cso-controller-manager created
certificate.cert-manager.io/cso-serving-cert created
issuer.cert-manager.io/cso-selfsigned-issuer created

kranurag7 · 2024-02-12T19:22:44Z

The actual error seems to be the following.

apiVersion: v1
kind: Secret
metadata:
  name: mysecret
type: Opaque
data:
  stuff: ${GIT_ACCESS_TOKEN_B64}

When we apply the above yaml we get the following error.

$ kubectl apply -f foo-secret.yaml
Error from server (BadRequest): error when creating "foo-secret.yaml": Secret in version "v1" cannot be handled as a Secret: illegal base64 data at input byte 0

The error exactly matches what we have on tilt UI above.

With above being confirmed I think the problem is with the substitution and pipes in the following lines.

    yaml_cso = './cso-components.yaml'
    local_resource(
        name = "cso-components",
        cmd = ["sh", "-ec", sed_cmd, yaml_cso, "|", envsubst_cmd],
        labels = ["CSO"],
    )

I'll give it another attempt tomorrow. If you know the solution already then please comment or push a commit on this branch.

chess-knight · 2024-02-13T06:39:43Z

I modified my deploy_cso() according to yours and I am getting Error in k8s_yaml: error reading yaml file: open ./cso-components.yaml: no such file or directory.

michal-gubricky · 2024-02-13T07:26:27Z

I modified my deploy_cso() according to yours and I am getting Error in k8s_yaml: error reading yaml file: open ./cso-components.yaml: no such file or directory.

That is because there is missing local(cmd, quiet = True) after cmd = "curl -sSL -o cso-components.yaml {}".format(cso_uri)

kranurag7 · 2024-02-13T08:16:22Z

Hey, the secret decoding is not working as of now. I will do the following now:

Cut new release of sovereigncloudstack/cluster-stack-operator with v0.1.0-alpha.3 and then I'll use the cso-components.yaml file that will not contain the secret. New release is required because local_mode in CSO was added after the v0.1.0-alpha.2 release.
We can do this because cso when used in local_mode doesn't require secret. Check the following lines:

https://github.com/SovereignCloudStack/cluster-stack-operator/blob/adb648ceaebddca04a015fbea0319110ca99a5cc/cmd/main.go#L123-L124

https://github.com/SovereignCloudStack/cluster-stack-operator/blob/adb648ceaebddca04a015fbea0319110ca99a5cc/pkg/github/client/credentials.go#L49-L75

Once this is done, we can improve on it. But in the current state secret decoding is not working and the alternative method suggested above is what my next plan is.

chess-knight · 2024-02-13T09:29:30Z

I probably found a problem with envsubst. It is not a problem with that at all. But I think that when you execute k8s_yaml(yaml_cso) you are referencing yaml_cso as a path to a file and not as the content of that file, which is IMO huge difference.

kranurag7 · 2024-02-13T09:35:39Z

I tried str(read_file('cso-components.yaml') and some more combination and there were errors.

The current method works when I tested by passing the config same way, I'm doing now.

I'll push the changes today after the release and testing. If it breaks we can iterate on it.

matofeder · 2024-02-23T13:21:02Z

@kranurag7
as the related work #88 has been merged, I'm wondering whether we can close this draft,

kranurag7 · 2024-02-23T14:45:26Z

Yes @matofeder We can close this one.

I have also updated the docs there. I think we can close this one.
I'm closing this one, please ask if you've some questions w.r.t. local_mode or this PR.

kranurag7 added 2 commits February 6, 2024 14:40

use local_mode with cspo

9d07ba8

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

change copy target to /tmp/downloads

f6f9d7a

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

jschoone requested review from chess-knight, jschoone and michal-gubricky February 6, 2024 09:30

chess-knight reviewed Feb 6, 2024

View reviewed changes

Comment thread Tiltfile

Comment thread Tiltfile

Comment thread Tiltfile Outdated

kranurag7 and others added 2 commits February 6, 2024 17:00

Update Tiltfile

5606f6e

Co-authored-by: Roman Hros <roman.hros@dnation.cloud> Signed-off-by: kranurag7 <81210977+kranurag7@users.noreply.github.com>

add docs and update gitignore

f2bdaf3

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

janiskemper suggested changes Feb 6, 2024

View reviewed changes

implement docs feedback

b7f50e1

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

chess-knight reviewed Feb 7, 2024

View reviewed changes

Comment thread docs/develop.md

mark local_mode to false in Tiltfile

c236395

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

chess-knight reviewed Feb 7, 2024

View reviewed changes

kranurag7 changed the title ~~✨ use local_mode with cspo~~ ✨ [WIP] use local_mode with cspo Feb 7, 2024

move cso deployment along with cspo in the repo

807ffa7

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

michal-gubricky suggested changes Feb 9, 2024

View reviewed changes

chess-knight reviewed Feb 9, 2024

View reviewed changes

address requested changes and more

a5b406d

Signed-off-by: kranurag7 <anurag.kumar@syself.com>

kranurag7 mentioned this pull request Feb 9, 2024

🐛 update to v2 release of openstack-scs clusterstack #83

Closed

michal-gubricky suggested changes Feb 12, 2024

View reviewed changes

chess-knight reviewed Feb 12, 2024

View reviewed changes

Update Tiltfile

5bf34a8

Co-authored-by: Michal Gubricky <michal.gubricky@dnation.cloud> Signed-off-by: kranurag7 <81210977+kranurag7@users.noreply.github.com>

kranurag7 closed this Feb 23, 2024

	yaml_cso = str(local(cmd, quiet = True))
	local(cmd, quiet = True)

Uh oh!

Conversation

kranurag7 commented Feb 6, 2024

Uh oh!

janiskemper commented Feb 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chess-knight Feb 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chess-knight Feb 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

janiskemper commented Feb 6, 2024 •

edited

Loading

chess-knight Feb 9, 2024 •

edited

Loading

chess-knight Feb 9, 2024 •

edited

Loading

kranurag7 commented Feb 12, 2024 •

edited

Loading