-
Notifications
You must be signed in to change notification settings - Fork 706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Retrieve every k8s resource generated by a Carvel package installation (alt) #4068
Changes from all commits
e1d6b2e
30852cc
c959008
513a249
8bada4e
6c50339
c000501
bb4337d
6072669
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,13 @@ import ( | |
"context" | ||
"fmt" | ||
|
||
"github.com/cppforlife/go-cli-ui/ui" | ||
ctlapp "github.com/k14s/kapp/pkg/kapp/app" | ||
kappcmdapp "github.com/k14s/kapp/pkg/kapp/cmd/app" | ||
kappcmdcore "github.com/k14s/kapp/pkg/kapp/cmd/core" | ||
kappcmdtools "github.com/k14s/kapp/pkg/kapp/cmd/tools" | ||
"github.com/k14s/kapp/pkg/kapp/logger" | ||
ctlres "github.com/k14s/kapp/pkg/kapp/resources" | ||
"github.com/kubeapps/kubeapps/cmd/kubeapps-apis/core" | ||
corev1 "github.com/kubeapps/kubeapps/cmd/kubeapps-apis/gen/core/packages/v1alpha1" | ||
"github.com/kubeapps/kubeapps/cmd/kubeapps-apis/gen/plugins/kapp_controller/packages/v1alpha1" | ||
|
@@ -26,6 +33,7 @@ import ( | |
) | ||
|
||
type clientGetter func(context.Context, string) (kubernetes.Interface, dynamic.Interface, error) | ||
type kappClientsGetter func(ctx context.Context, cluster, namespace string) (ctlapp.Apps, ctlres.IdentifiedResources, *kappcmdapp.FailingAPIServicesPolicy, ctlres.ResourceFilter, error) | ||
|
||
const ( | ||
globalPackagingNamespace = "kapp-controller-packaging-global" | ||
|
@@ -43,6 +51,7 @@ type Server struct { | |
clientGetter clientGetter | ||
globalPackagingNamespace string | ||
globalPackagingCluster string | ||
kappClientsGetter kappClientsGetter | ||
} | ||
|
||
// NewServer returns a Server automatically configured with a function to obtain | ||
|
@@ -69,6 +78,50 @@ func NewServer(configGetter core.KubernetesConfigGetter, globalPackagingCluster | |
}, | ||
globalPackagingNamespace: globalPackagingNamespace, | ||
globalPackagingCluster: globalPackagingCluster, | ||
kappClientsGetter: func(ctx context.Context, cluster, namespace string) (ctlapp.Apps, ctlres.IdentifiedResources, *kappcmdapp.FailingAPIServicesPolicy, ctlres.ResourceFilter, error) { | ||
if configGetter == nil { | ||
return ctlapp.Apps{}, ctlres.IdentifiedResources{}, nil, ctlres.ResourceFilter{}, status.Errorf(codes.Internal, "configGetter arg required") | ||
} | ||
// Retrieve the k8s REST client from the configGetter | ||
config, err := configGetter(ctx, cluster) | ||
if err != nil { | ||
return ctlapp.Apps{}, ctlres.IdentifiedResources{}, nil, ctlres.ResourceFilter{}, status.Errorf(codes.FailedPrecondition, "unable to get config due to: %v", err) | ||
} | ||
// Pass the REST client to the (custom) kapp factory | ||
configFactory := NewConfigurableConfigFactoryImpl() | ||
configFactory.ConfigureRESTConfig(config) | ||
Comment on lines
+90
to
+92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like a simple and useful change, so not sure why not. |
||
depsFactory := kappcmdcore.NewDepsFactoryImpl(configFactory, ui.NewNoopUI()) | ||
|
||
// Create an empty resource filter | ||
resourceFilterFlags := kappcmdtools.ResourceFilterFlags{} | ||
resourceFilter, err := resourceFilterFlags.ResourceFilter() | ||
if err != nil { | ||
return ctlapp.Apps{}, ctlres.IdentifiedResources{}, nil, ctlres.ResourceFilter{}, status.Errorf(codes.FailedPrecondition, "unable to get config due to: %v", err) | ||
} | ||
|
||
// Create the preconfigured resource types flags and a failing policy | ||
resourceTypesFlags := kappcmdapp.ResourceTypesFlags{ | ||
// Allow to ignore failing APIServices | ||
IgnoreFailingAPIServices: true, | ||
// Scope resource searching to fallback allowed namespaces | ||
ScopeToFallbackAllowedNamespaces: true, | ||
} | ||
failingAPIServicesPolicy := resourceTypesFlags.FailingAPIServicePolicy() | ||
|
||
// 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) | ||
} | ||
Comment on lines
+111
to
+121
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, sorry, let me explain myself better. When we install a package in Kubeapps in a certain namespace (e.g., Harbor in However, when reconciling, this Perhaps there's something I missed in the Kapp CLI code. In fact, they have a |
||
|
||
return supportingNsObjs.Apps, supportingObjs.IdentifiedResources, failingAPIServicesPolicy, resourceFilter, nil | ||
}, | ||
} | ||
} | ||
|
||
|
@@ -83,3 +136,15 @@ func (s *Server) GetClients(ctx context.Context, cluster string) (kubernetes.Int | |
} | ||
return typedClient, dynamicClient, nil | ||
} | ||
|
||
// GetKappClients ensures a client getter is available and uses it to return a Kapp Factory. | ||
func (s *Server) GetKappClients(ctx context.Context, cluster, namespace string) (ctlapp.Apps, ctlres.IdentifiedResources, *kappcmdapp.FailingAPIServicesPolicy, ctlres.ResourceFilter, error) { | ||
if s.clientGetter == nil { | ||
return ctlapp.Apps{}, ctlres.IdentifiedResources{}, nil, ctlres.ResourceFilter{}, status.Errorf(codes.Internal, "server not configured with configGetter") | ||
} | ||
appsClient, resourcesClient, failingAPIServicesPolicy, resourceFilter, err := s.kappClientsGetter(ctx, cluster, namespace) | ||
if err != nil { | ||
return ctlapp.Apps{}, ctlres.IdentifiedResources{}, nil, ctlres.ResourceFilter{}, status.Errorf(codes.FailedPrecondition, fmt.Sprintf("unable to get Kapp Factory : %v", err)) | ||
} | ||
return appsClient, resourcesClient, failingAPIServicesPolicy, resourceFilter, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -249,8 +249,6 @@ func (s *Server) buildInstalledPackageDetail(pkgInstall *packagingv1alpha1.Packa | |
deployStderr := "" | ||
fetchStdout := "" | ||
fetchStderr := "" | ||
inspectStdout := "" | ||
inspectStderr := "" | ||
|
||
if app.Status.Deploy != nil { | ||
deployStdout = app.Status.Deploy.Stdout | ||
|
@@ -260,10 +258,6 @@ func (s *Server) buildInstalledPackageDetail(pkgInstall *packagingv1alpha1.Packa | |
fetchStdout = app.Status.Fetch.Stdout | ||
fetchStderr = app.Status.Fetch.Stderr | ||
} | ||
if app.Status.Inspect != nil { | ||
inspectStdout = app.Status.Inspect.Stdout | ||
inspectStderr = app.Status.Inspect.Stderr | ||
} | ||
Comment on lines
-263
to
-266
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing the text-plain output of the resources, as it becomes redundant after this PR. |
||
|
||
// Build some custom installation notes based on the available stdout + stderr | ||
// TODO(agamez): this is just a temporary solution until come up with a better UX solution | ||
|
@@ -279,10 +273,6 @@ func (s *Server) buildInstalledPackageDetail(pkgInstall *packagingv1alpha1.Packa | |
%s | ||
|
||
|
||
### Inspect: | ||
%s | ||
|
||
|
||
## Errors | ||
|
||
|
||
|
@@ -294,10 +284,7 @@ func (s *Server) buildInstalledPackageDetail(pkgInstall *packagingv1alpha1.Packa | |
%s | ||
|
||
|
||
### Inspect: | ||
%s | ||
|
||
`, deployStdout, fetchStdout, inspectStdout, deployStderr, fetchStderr, inspectStderr) | ||
`, deployStdout, fetchStdout, deployStderr, fetchStderr) | ||
|
||
if len(pkgInstall.Status.Conditions) > 1 { | ||
log.Warningf("The package install %s has more than one status conditions. Using the first one: %s", pkgInstall.Name, pkgInstall.Status.Conditions[0]) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,9 @@ package main | |
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
corev1 "github.com/kubeapps/kubeapps/cmd/kubeapps-apis/gen/core/packages/v1alpha1" | ||
kappctrlv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" | ||
packagingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" | ||
datapackagingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1" | ||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is mostly the code that kapp CLI itself uses for inspecting an app, but adapted to our needs. |
||
// As per https://github.com/vmware-tanzu/carvel-kapp-controller/blob/v0.31.0/pkg/deploy/kapp.go#L151 | ||
appName := fmt.Sprintf("%s-ctrl", packageId) | ||
|
||
refs := []*corev1.ResourceRef{} | ||
|
||
// Get the Kapp different clients | ||
appsClient, resourcesClient, failingAPIServicesPolicy, _, err := s.GetKappClients(ctx, cluster, namespace) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// Fetch the Kapp App | ||
app, err := appsClient.Find(appName) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// Fetch the GroupVersions used by the app | ||
usedGVs, err := app.UsedGVs() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// Mark those GVs as required | ||
failingAPIServicesPolicy.MarkRequiredGVs(usedGVs) | ||
|
||
// Create a k8s label selector for the app | ||
labelSelector, err := app.LabelSelector() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// List the k8s resources that match the label selector | ||
resources, err := resourcesClient.List(labelSelector, nil) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// For each resource, generate and append the ResourceRef | ||
for _, resource := range resources { | ||
refs = append(refs, &corev1.ResourceRef{ | ||
ApiVersion: resource.GroupVersion().String(), | ||
Kind: resource.Kind(), | ||
Name: resource.Name(), | ||
Namespace: resource.Namespace(), | ||
}) | ||
} | ||
return refs, nil | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Excellent, thanks for the very clear code in this part :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.