Deploy a chart via flux #3640

absoludity · 2021-10-22T04:06:31Z

Description of the change

Similar to the last PR #3628 , this one updates the deploymentform routes so that the available package's namespace is referenced explicitly, rather than relying on a global or namespaced repo.

With this change, I'm able to deploy bitnami apache (pod only, see below) via Flux! Woohoo! But before you get too excited, it just brings us to the next point of integration excitement. See the additional notes below.

Benefits

One step closer to fluxv2 UX integration.

Possible drawbacks

Applicable issues

Ref Enable flux plugin to be used from Kubeapps UX #3610

Additional information

The UX fails to find the release that it just tried to create in the default namespace:

It appears that although my Apache pod is appearing in the correct namespace (default), the HelmRelease resource is created in the kubeapps namespace, rather than the default namespace:

$ kubectl get po -n default
NAME                                         READY   STATUS    RESTARTS   AGE
default-test-apache-flux-2-897565695-wddtb   1/1     Running   0          2m58s

$ kubectl get helmrelease -A
NAMESPACE   NAME                 READY     STATUS                       AGE
kubeapps    test-apache-flux-2   Unknown   Reconciliation in progress   3m22s

Checking the api server logs shows that it's receiving the request to create the installed package in the default namespace:

I1022 03:22:47.706471       1 packages.go:261] +core CreateInstalledPackage (cluster="default", namesp
ace="default")
I1022 03:22:47.706489       1 server.go:364] +fluxv2 CreateInstalledPackage [available_package_ref:{co
ntext:{namespace:"flux-system"} identifier:"bitnami%2Fapache" plugin:{name:"fluxv2.packages" version:"
v1alpha1"}} target_context:{cluster:"default" namespace:"default"} name:"test-via-flux" pkg_version_re
ference:{version:"8.8.4"} values: ...

I'm assuming this isn't intended, but I'll let @gfichtenholt take a look before I investigate more.

There are other issues in the UX which we'll need to deal with next too. In particular, the UX is immediately trying to request the helm release (via the old kubeops service - this was unexpected), which (a) won't exist immediately and (b) the UX can't assume a helm release is created, we should only be getting installed packages. In my case, the helm release fails as does the flux reconciliation, although the apache pod was created - need to dig more there.

$ helm ls -A
 NAME                           NAMESPACE               REVISION        UPDATED                                       STATUS          CHART                   APP VERSION
default-test-apache-flux-2      default                 1               2021-10-22 03:49:29.736340602 +0000 UTC       failed          apache-8.8.4            2.4.51
...

$ kubectl get helmrelease -A
NAMESPACE   NAME                 READY   STATUS                      AGE
kubeapps    test-apache-flux-2   False   install retries exhausted   16m

gfichtenholt · 2021-10-22T04:18:08Z

I'm assuming this isn't intended, but I'll let @gfichtenholt take a look before I investigate more.

yeah, that was kind of intended. For the 'CreateInstalledPackage' operation I have potentially up to 3 different namespaces to work with:

that of the helmrepository (and helmchart) CR
that of the helmrelease CR
target namespace (where the pod will go)
I arbitrarily chose (2) to be kubeapps namespace for now
https://github.com/kubeapps/kubeapps/blob/master/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/release.go#L334
Open to suggestions for a better way :-)

absoludity · 2021-10-22T06:09:47Z

yeah, that was kind of intended. For the 'CreateInstalledPackage' operation I have potentially up to 3 different namespaces to work with:

1. that of the helmrepository (and helmchart) CR

2. that of the helmrelease CR

3. target namespace (where the pod will go)
   I arbitrarily chose (2) to be kubeapps namespace for now
   https://github.com/kubeapps/kubeapps/blob/master/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/release.go#L334
   Open to suggestions for a better way :-)

Yeah, OK. So yes, 3 is obvious - the resources should be in the target namespace, but I'd be keen for (2) the helm release CR to also be created in the target namespace (where the helm release itself is currently created, as shown above), if that also makes sense to you?

I mean, the API request is to create an installed package in the default namespace, but what is returned by the API is an installed package in the kubeapps namespace, which is going to be very confusing from a UX point of view (in fact, I don't know that we can even represent that in a sane way... the installed package / HelmRelease CR got created in the kubeapps namespace, the actual helm release is listed as being in the default namespace where the resources are). So yes, if you're OK with that, +1 to change that so the installed package / helm release CR is created in the target namespace indicated in the request?

gfichtenholt · 2021-10-22T06:13:16Z

So yes, if you're OK with that, +1 to change that so the installed package / helm release CR is created in the target namespace indicated in the request?

I am ok with that. Feel free to make that change if you like or I can make it. Whatever lets you go faster

absoludity · 2021-10-22T06:40:56Z

So yes, if you're OK with that, +1 to change that so the installed package / helm release CR is created in the target namespace indicated in the request?

I am ok with that. Feel free to make that change if you like or I can make it. Whatever lets you go faster

Thanks. If you can do the fluxv2 change, I'll continue with the dashboard - that'd be great.

gfichtenholt · 2021-10-22T06:58:22Z

So yes, if you're OK with that, +1 to change that so the installed package / helm release CR is created in the target namespace indicated in the request?

I am ok with that. Feel free to make that change if you like or I can make it. Whatever lets you go faster

Thanks. If you can do the fluxv2 change, I'll continue with the dashboard - that'd be great.

No problem. Follow-up question: if the target namespace does not exist (fairly common use case), should I create it programmatically inside the 'CreateInstalledPackage" call or let it fail? I do currently set the "createNamespace" flag to true in flux HelmRelease CRD (another arbitrary decision by me), but this change is going to make that decision irrelevant - I have to be able to place the CRD into a namespace first, before flux can even do its magic. Or yet a 3rd option would be to leave that decision to the caller. We'd need to add a flag 'create_namespace' to our CreateInstalledPackageRequest, if caller says 'true' I do it, otherwise fail if the namespace doesn't exist

antgamdia

Awesome, thanks for the changes here!

antgamdia · 2021-10-22T08:18:20Z

dashboard/src/components/PackageHeader/PackageView.tsx

-        } as AvailablePackageReference,
-        packageVersion,
-      ),
+      actions.packages.fetchAndSelectAvailablePackageDetail(packageReference, packageVersion),


absoludity · 2021-10-24T19:42:47Z

No problem. Follow-up question: if the target namespace does not exist (fairly common use case), should I create it programmatically inside the 'CreateInstalledPackage" call or let it fail?

For now, I'd let it fail as a bad request. It doesn't affect our UX since the create request is always in the context of a namespace that is already created. And longer term...

I do currently set the "createNamespace" flag to true in flux HelmRelease CRD (another arbitrary decision by me), but this change is going to make that decision irrelevant - I have to be able to place the CRD into a namespace first, before flux can even do its magic. Or yet a 3rd option would be to leave that decision to the caller. We'd need to add a flag 'create_namespace' to our CreateInstalledPackageRequest, if caller says 'true' I do it, otherwise fail if the namespace doesn't exist

Yes, longer term we can support this if there is a need. But for now, +1 to fail with a bad request.

Thanks Greg.

Signed-off-by: Michael Nelson <minelson@vmware.com>

gfichtenholt · 2021-10-25T05:59:05Z

For now, I'd let it fail as a bad request

Sounds good. Will do that. What is gRPC code for 'bad request'? I don't see one
https://pkg.go.dev/google.golang.org/grpc/codes
According to this it should be Internal. Just wanted to verify this is what you'd expect

* step 1 * step 2 * step 3 * step 4 * Michael's feedback #1

absoludity mentioned this pull request Oct 22, 2021

Enable flux plugin to be used from Kubeapps UX #3610

Closed

7 tasks

antgamdia approved these changes Oct 22, 2021

View reviewed changes

Base automatically changed from flux-ui-3 to master October 24, 2021 21:14

absoludity added 2 commits October 25, 2021 08:17

Deploy via flux.

c4e39ba

Signed-off-by: Michael Nelson <minelson@vmware.com>

Lint

989031d

Signed-off-by: Michael Nelson <minelson@vmware.com>

absoludity force-pushed the flux-ui-4 branch from ff8e7dc to 989031d Compare October 24, 2021 21:17

absoludity merged commit c1d6743 into master Oct 24, 2021

absoludity deleted the flux-ui-4 branch October 24, 2021 22:38

gfichtenholt mentioned this pull request Oct 26, 2021

flux plugin to use targetNamespace for HelmRelease CRD per #3640 #3662

Merged

gfichtenholt added a commit that referenced this pull request Oct 27, 2021

flux plugin to use targetNamespace for HelmRelease CRD per #3640 (#3662)

4e9040a

* step 1 * step 2 * step 3 * step 4 * Michael's feedback #1

This was referenced Jan 27, 2022

Don't set explicit targetNamespace for HelmRelease. #4172

Closed

Cannot view newly created flux 2 package from UX #4173

Closed

absoludity mentioned this pull request Apr 6, 2022

[Regression] Allow package details to fetch from config.kubeappsCluster #4564

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deploy a chart via flux #3640

Deploy a chart via flux #3640

absoludity commented Oct 22, 2021

gfichtenholt commented Oct 22, 2021 •

edited

Loading

absoludity commented Oct 22, 2021

gfichtenholt commented Oct 22, 2021

absoludity commented Oct 22, 2021

gfichtenholt commented Oct 22, 2021 •

edited

Loading

antgamdia left a comment

antgamdia Oct 22, 2021

absoludity commented Oct 24, 2021

gfichtenholt commented Oct 25, 2021 •

edited

Loading

Deploy a chart via flux #3640

Deploy a chart via flux #3640

Conversation

absoludity commented Oct 22, 2021

Description of the change

Benefits

Possible drawbacks

Applicable issues

Additional information

gfichtenholt commented Oct 22, 2021 • edited Loading

absoludity commented Oct 22, 2021

gfichtenholt commented Oct 22, 2021

absoludity commented Oct 22, 2021

gfichtenholt commented Oct 22, 2021 • edited Loading

antgamdia left a comment

Choose a reason for hiding this comment

antgamdia Oct 22, 2021

Choose a reason for hiding this comment

absoludity commented Oct 24, 2021

gfichtenholt commented Oct 25, 2021 • edited Loading

gfichtenholt commented Oct 22, 2021 •

edited

Loading

gfichtenholt commented Oct 22, 2021 •

edited

Loading

gfichtenholt commented Oct 25, 2021 •

edited

Loading