Retrieve every k8s resource generated by a Carvel package installation (alt) #4068

antgamdia · 2022-01-12T20:56:36Z

(Alternative solution for the same purpose of #4062)

Description of the change

This PR removes the workaround we implemented when we released it last time and implements a generic way to retrieve all the kubernetes resources matching a certain label. This way we can fetch every k8s resource created as a result of a Carvel PackageInstall.

Benefits

The retrieved resources for a carvel package will now include every possible one, not a fixed list of pods, svc and deployments.

New: it uses the Kapp code directly, instead of manually iterating over the k8s resources manually.

Possible drawbacks

Hitting the k8s APIs for querying every possible available resource is expensive in time if the QPS is low. Using a reasonable value (as OpenShift does) of 50 QPS, the waiting time decreases.

Applicable issues

fixes Add a proper implementation for GetInstalledPackageResourceRefs #3854
requires Fix namespace when watching (rather than getting) resources. #4057 for the UI to be fully working.

Additional information

** Testing pending, I'm not able to get it to work so far **

PR in action:

carvelResourcesKapp2.mp4

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

antgamdia · 2022-01-13T18:54:17Z

chart/kubeapps/values.yaml

+  qps: "50.0"
  ## @param kubeappsapis.burst KubeappsAPIs Kubernetes API client Burst limit
  ##
-  burst: "15"
+  burst: "100"


Limits increased with the same OpenShift defaults; the waiting time is now reasonable, though increasing the QPS will of course benefit the response time when querying the k8s API.

antgamdia · 2022-01-13T18:55:59Z

cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server.go

+			// Pass the REST client to the (custom) kapp factory
+			configFactory := NewConfigurableConfigFactoryImpl()
+			configFactory.ConfigureRESTConfig(config)


This is the part currently unsupported by the Carvel team. They recommended we just implement our custom struct satisfying their interface, don't know if they will be willing to accept a PR for that, though.

Seems like a simple and useful change, so not sure why not.

antgamdia · 2022-01-13T18:57:57Z

cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server.go

+			// Getting namespaced clients (e.g., for fetching an App)
+			supportingNsObjs, err := kappcmdapp.FactoryClients(depsFactory, kappcmdcore.NamespaceFlags{Name: namespace}, resourceTypesFlags, logger.NewNoopLogger())
+			if err != nil {
+				return ctlapp.Apps{}, ctlres.IdentifiedResources{}, nil, ctlres.ResourceFilter{}, status.Errorf(codes.FailedPrecondition, "unable to get config due to: %v", err)
+			}
+
+			// Getting non-namespaced clients (e.g., for fetching every k8s object in the cluster)
+			supportingObjs, err := kappcmdapp.FactoryClients(depsFactory, kappcmdcore.NamespaceFlags{Name: ""}, resourceTypesFlags, logger.NewNoopLogger())
+			if err != nil {
+				return ctlapp.Apps{}, ctlres.IdentifiedResources{}, nil, ctlres.ResourceFilter{}, status.Errorf(codes.FailedPrecondition, "unable to get config due to: %v", err)
+			}


This is the only way I've seen to able to query the resources that have been created in a different namespace (like Harbor). I can't find any reference to this freshly created namespace in any of the kapp-related CRs.

Sorry, I'm missing some info here? Does the harbor package create resources in the target namespace (of the package install) as well as another namespace? And why can't we see the client that the Kapp CLI creates to do the same command? (which you've mentioned below, just not sure why we can't also see the same for this issue here).

Yep, sorry, let me explain myself better.

When we install a package in Kubeapps in a certain namespace (e.g., Harbor in default ns), we are creating: a PackageInstall in default and a ConfigMap in default.
Additionally, kapp, under the scenes, is gonna create an App in default. Neither the PackageInstall nor the App has references to anything different from the namespace default.

However, when reconciling, this App starts creating the resources, which happen to be a namespace harbor and a set of several resources that are also created in this harbor namespace.

Perhaps there's something I missed in the Kapp CLI code. In fact, they have a --dangerous-scope-to-fallback-allowed-namespaces flag, so I guess, for the name it has, they are not allowing querying everything.
If only I had a reference to this harbor namespace somewhere, I'd be able to query the current default ns as well as harbor as this kind of "fallback allowed namespace".

antgamdia · 2022-01-13T18:58:44Z

cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_adapters.go

-	if app.Status.Inspect != nil {
-		inspectStdout = app.Status.Inspect.Stdout
-		inspectStderr = app.Status.Inspect.Stderr
-	}


Removing the text-plain output of the resources, as it becomes redundant after this PR.

antgamdia · 2022-01-13T19:00:13Z

cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_resources.go

@@ -399,3 +401,55 @@ func (s *Server) updatePkgInstall(ctx context.Context, cluster, namespace string
 	}
 	return &pkgInstall, nil
 }
+
+// inspectKappK8sResources returns the list of k8s resources matching the given listOptions
+func (s *Server) inspectKappK8sResources(ctx context.Context, cluster, namespace, packageId string) ([]*corev1.ResourceRef, error) {


This is mostly the code that kapp CLI itself uses for inspecting an app, but adapted to our needs.

antgamdia · 2022-01-13T19:02:24Z

cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_test.go

+				kappClientsGetter: func(ctx context.Context, cluster, namespace string) (ctlapp.Apps, ctlres.IdentifiedResources, *kappcmdapp.FailingAPIServicesPolicy, ctlres.ResourceFilter, error) {
+					// Create a fake DepsFactory and pass it the fake k8s clients
+					depsFactory := NewFakeDepsFactoryImpl()
+					depsFactory.SetCoreClient(typedClient)
+					depsFactory.SetDynamicClient(dynClient)


This is the approach I've been able to come up with for faking the kapp internal clients. Even if it kinda works, it returns an empty list of resources found :(
If you have any suggestions or can give it a try, I'd really appreciate it!

So I setup the vscode debugger (which was pretty simple - hadn't used it much before) and traced this down from your call to resources, err := resourcesClient.List(labelSelector, nil) in server_data_resources.go. From there:

kapp first tries to list all the resource types on the cluster, which, for your test client, results in an empty slice, so that

kapp goes on to call resources.All(resTypes, allOpts), but this just iterates the slice of resTypes, so also returns an empty slice.

Looking at the implementation, the core issue is the function which collects the resource types is using the discovery api, so in your test it's not returning any APIs. Might be worth re-checking the kapp test code to see how they setup the discovery API in tests or otherwise ensure certain resource types (pods, in your case) are returned.

HTH!

absoludity

+1ing in case I don't get back to it today, as it looks great. Hopefully I'll get to look at the issue for the tests later too.

absoludity · 2022-01-13T19:25:23Z

cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_resources.go

+		})
+	}
+	return refs, nil
+}


Excellent, thanks for the very clear code in this part :)

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

antgamdia · 2022-01-14T15:33:25Z

cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_test.go

+			typedClient := typfake.NewSimpleClientset(tc.existingTypedObjects...)
+
+			// We cast the dynamic client to a fake client, so we can set the response
+			fakeDiscovery, _ := typedClient.Discovery().(*disfake.FakeDiscovery)
+			fakeDiscovery.Fake.Resources = apiResources


After creating a bunch of custom types... it just came down to this approach.
The crux was just in casting the Discovery() result so that we can manually leverage from the builtin .Fake.Resources

Thanks for the pointer, Michael. Best solution when you're stuck is have some rest and start over the next day with a fresh mind :P

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> Conflicts: go.mod go.sum

antgamdia mentioned this pull request Jan 13, 2022

Retrieve every k8s resource generated by a Carvel package installation #4062

Closed

wip on Get k8s resources using kapp

e1d6b2e

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

antgamdia force-pushed the 3854-getResources-alt branch from 21d76e1 to e1d6b2e Compare January 13, 2022 13:34

antgamdia added 6 commits January 13, 2022 14:35

Update go deps

30852cc

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Inspect k8s resources in every ns

c959008

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Remove raw text output of the "inspect"

513a249

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Increase default qps/burt limits

8bada4e

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Remove unused code

6c50339

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Add (failing) TestGetInstalledPackageResourceRefs

c000501

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

antgamdia marked this pull request as ready for review January 13, 2022 18:52

antgamdia commented Jan 13, 2022

View reviewed changes

antgamdia mentioned this pull request Jan 13, 2022

Remaining work for running Kubeapps with Carvel #3849

Closed

21 tasks

absoludity approved these changes Jan 13, 2022

View reviewed changes

Make the test case work

bb4337d

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

antgamdia commented Jan 14, 2022

View reviewed changes

Merge branch 'master'

6072669

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> Conflicts: go.mod go.sum

antgamdia merged commit 106d417 into vmware-tanzu:master Jan 20, 2022

antgamdia deleted the 3854-getResources-alt branch January 20, 2022 16:01

antgamdia mentioned this pull request Jan 20, 2022

Wait until CM is created when installing a carvel pkg #4139

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Retrieve every k8s resource generated by a Carvel package installation (alt) #4068

Retrieve every k8s resource generated by a Carvel package installation (alt) #4068

antgamdia commented Jan 12, 2022 •

edited

antgamdia Jan 13, 2022

antgamdia Jan 13, 2022

absoludity Jan 13, 2022

antgamdia Jan 13, 2022

absoludity Jan 13, 2022 •

edited

antgamdia Jan 13, 2022

antgamdia Jan 13, 2022

antgamdia Jan 13, 2022

antgamdia Jan 13, 2022

absoludity Jan 13, 2022

absoludity left a comment

absoludity Jan 13, 2022

antgamdia Jan 14, 2022

Retrieve every k8s resource generated by a Carvel package installation (alt) #4068

Retrieve every k8s resource generated by a Carvel package installation (alt) #4068

Conversation

antgamdia commented Jan 12, 2022 • edited

Description of the change

Benefits

Possible drawbacks

Applicable issues

Additional information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

absoludity Jan 13, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

absoludity left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antgamdia commented Jan 12, 2022 •

edited

absoludity Jan 13, 2022 •

edited