This repository has been archived by the owner on Oct 10, 2023. It is now read-only.
TKR Source Controller: Install TKR package contents directly #2524
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
imikushin
added
ok-to-merge
Codecov Report
@@ Coverage Diff @@
## main #2524 +/- ##
==========================================
- Coverage 42.32% 42.32% -0.01%
==========================================
Files 398 398
Lines 39518 39692 +174
==========================================
+ Hits 16726 16798 +72
- Misses 21219 21308 +89
- Partials 1573 1586 +13
Continue to review full report at Codecov.
|
We're making PatchSet more generic and efficient. Before this change, PatchSet would not work with ConfigMaps. We're also removing a dependency on util/patch from Cluster API. PatchSet is also now safe for concurrent use. Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
imikushin
force-pushed
the
tkr-sc-no-pkgi
branch
2 times, most recently
from
abc34ce to
665627e
Compare
imikushin
added
kind/enhancement
TKR package contents get automatically installed without the use of Carvel PackageInstall resource (which doesn't support shared resources across packages). This is necessary because a TKR package may contain resources that are reused by other TKR packages, e.g. different TKRs shipping the same Kubernetes version may share some OSImages or addon packages (addon packages may even be reused across K8s versions). Inject Registry as an interface This improves testability and modularity of affected components, and reuse of Registry as a component. Now, both Fetcher and TKR Package Reconciler re-use the same Registry instance provided externally. Only fetch compatible TKR BOMs and TKR packages We're also now using compatibility information to download only compatible TKR packages and TKR BOMs into the management cluster. Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
prkalle
approved these changes
Jun 2, 2022
| if err := f.createTKRPackages(ctx, tag); err != nil { | ||
| errs = append(errs, errors.Wrapf(err, "failed to create TKR Package for image %s", fmt.Sprintf("%s:%s", f.Config.TKRRepoImagePath, tag))) | ||
| } | ||
| } | ||
| if len(errs) != 0 { | ||
| return errs | ||
| } | ||
|
|
||
| f.Log.Info("Done fetching TKR Packages", "image", f.Config.TKRRepoImagePath) |
There was a problem hiding this comment.
nit: This log in case of errors would be misleading?
imikushin
pushed a commit
to imikushin/tanzu-framework
that referenced
this pull request
Jun 7, 2022
Without this fix, `bom-metadata` ConfigMap in the `tkr-system` namespace would differ in `binaryData.compatibility` value from its contents prior to vmware-tanzu#2524. Adding a unit test that would fail illustrating the difference: ``` Expected <string>: "Manage..." to equal | <string>: "manage..." ``` Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
3 tasks
imikushin
added a commit
that referenced
this pull request
Jun 7, 2022
Without this fix, `bom-metadata` ConfigMap in the `tkr-system` namespace would differ in `binaryData.compatibility` value from its contents prior to #2524. Adding a unit test that would fail illustrating the difference: ``` Expected <string>: "Manage..." to equal | <string>: "manage..." ``` Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
ankeesler
pushed a commit
to ankeesler/tanzu-framework
that referenced
this pull request
Jun 14, 2022
…zu#2559) Without this fix, `bom-metadata` ConfigMap in the `tkr-system` namespace would differ in `binaryData.compatibility` value from its contents prior to vmware-tanzu#2524. Adding a unit test that would fail illustrating the difference: ``` Expected <string>: "Manage..." to equal | <string>: "manage..." ``` Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
# for free
to subscribe to this conversation on GitHub.
Already have an account?
#.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it
Install TKR package contents directly
TKR package contents get automatically installed without the use of Carvel PackageInstall resource (which doesn't support shared resources across packages). This is necessary because a TKR package may contain resources that are reused by other TKR packages, e.g. different TKRs shipping the same Kubernetes version may share some OSImages or addon packages (addon packages may even be reused across K8s versions).
Inject Registry as an interface into components that need it
This improves testability and modularity of affected components, and reuse of Registry as a component. Now, both Fetcher and TKR Package Reconciler re-use the same Registry instance provided externally.
Only fetch compatible TKR BOMs and TKR packages
We're also now using compatibility information to download only compatible TKRs into the management cluster.
PatchSet improvements
We're making PatchSet more generic and efficient.
Before this change, PatchSet would not work with ConfigMaps. We're also removing a dependency on util/patch from Cluster API.
PatchSet is also now safe for concurrent use.
Which issue(s) this PR fixes
N/A
Describe testing done for PR
Release note
PR Checklist
Additional information
Special notes for your reviewer