Fix timeout for Carvel installed package summaries #4607
Conversation
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
| @@ -163,7 +163,7 @@ func (s *Server) buildInstalledPackageSummary(pkgInstall *packagingv1alpha1.Pack | |||
| IconUrl: iconStringBuilder.String(), | |||
| InstalledPackageRef: &corev1.InstalledPackageReference{ | |||
| Context: &corev1.Context{ | |||
| Namespace: pkgMetadata.Namespace, | |||
| Namespace: pkgInstall.Namespace, | |||
There was a problem hiding this comment.
Previously the package metadata was requested in the same namespace (individually) as the package install, and the aggregate API (remember, not a CR), returns a result with that same namespace, so they were equivalent. Now that we're fetching all package metadatas once (potentially across all namespaces if requested by the user), the pkgMetadata.Namespace might be the global carvel one, rather than the one we're after for the installed package (ie. they can differ now).
|
I did a test successfully on TCE 0.11 (= kubernetes 1.23.4 )after doing a build of images using the makefile of this PR
|
Excellent, thanks for checking @cmoulliard . That's still only a handful of installed apps, but it's definitely much more responsive. |
| versions map[string]*datapackagingv1alpha1.Package | ||
| } | ||
| pkgDatas := make(map[string]map[string]*pkgMetaAndVersionsData) | ||
| for _, pi := range pkgInstalls { |
There was a problem hiding this comment.
I have a slight preference for using the singular name (pkgInstall) instead of acronyms, but not doesn't really matter.
There was a problem hiding this comment.
Yeah, I do too... I think I initially was having naming issues and thought this would just be used over 2 lines, but it's not. I'll update before landing.
Signed-off-by: Michael Nelson <minelson@vmware.com>
Description of the change
I thought this was going to be similar to #4378 but although a similar approach can be taken, the solution is quite different, due to the iteration of package installs (rather than metadata).
It also ended up substantially more complicated for two reasons that I've highlighted in the code with a comment.
Benefits
Response times are approximately 1/6th for fetching installed package summaries (tested locally with 10 installed packages).
Possible drawbacks
Applicable issues
Additional information