Skip to content
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

Feat(#5861): Support addon dependencies version ranges #6002

Merged
merged 24 commits into from
Jun 7, 2023
Merged

Feat(#5861): Support addon dependencies version ranges #6002

merged 24 commits into from
Jun 7, 2023

Conversation

merusso
Copy link
Contributor

@merusso merusso commented May 16, 2023

Description of your changes

This change enables addon maintainers to define version ranges for dependencies in an addon's metadata.yaml file.

This behavior is similar to the version range allowed in the system section of the metadata file. The version range expression for dependencies follows the same format as for system.

Example:

dependencies:
  - name: addon1
    version: ">= 2.3.3, < 3.0.0"
  - name: addon2
    version: ">= 0.1.0, < 1.0.0"

When installing an addon, the behavior varies depending on whether the dependency is already installed.

If a dependency is already installed, the installed version will be validated against the version range, and installation will fail with an error if there's a mismatch.
If a dependency is not installed, the version range will be used to select the addon version to be installed. If no addon version matching the range exists, the installation will fail with an error.

Fixes #5861

I have:

How has this code been tested

Added unit tests for new functions.
Manually tested on an existing cluster.

Special notes for your reviewer

This change enables addon maintainers to define version ranges for
dependencies in an addon's metadata.yaml file.

This behavior is similar to the version range allowed in the `system`
section of the metadata file. The version range expression for
`dependencies` follows the same format as for `system`.

Example:

```yaml
dependencies:
  - name: addon1
    version: ">= 2.3.3, < 3.0.0"
  - name: addon2
    version: ">= 0.1.0, < 1.0.0"
```

When installing an addon, the behavior varies depending on whether the
dependency is already installed.

If a dependency is already installed, the installed version will be
validated against the version range, and installation will fail with an
error if there's a mismatch.
If a dependency is not installed, the version range will be used to
select the addon version to be installed. If no addon version matching
the range exists, the installation will fail with an error.

Fixes #5861

Signed-off-by: Michael Russo <merusso@gmail.com>
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Patch coverage: 72.95% and project coverage change: +0.50 🎉

Comparison is base (7a8264d) 66.04% compared to head (e433245) 66.54%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6002      +/-   ##
==========================================
+ Coverage   66.04%   66.54%   +0.50%     
==========================================
  Files         193      191       -2     
  Lines       25127    24851     -276     
==========================================
- Hits        16595    16538      -57     
+ Misses       6917     6711     -206     
+ Partials     1615     1602      -13     
Flag Coverage Δ
core-unittests 55.17% <72.95%> (+0.30%) ⬆️
e2e-multicluster-test 30.73% <0.00%> (+0.03%) ⬆️
e2etests 33.58% <ø> (+0.43%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/addon/source.go 50.45% <45.00%> (-1.22%) ⬇️
pkg/addon/addon.go 62.66% <78.62%> (+1.95%) ⬆️
pkg/addon/utils.go 67.71% <100.00%> (-7.87%) ⬇️
pkg/addon/versioned_registry.go 80.00% <100.00%> (+1.11%) ⬆️

... and 30 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
pkg/addon/addon.go Outdated Show resolved Hide resolved
Signed-off-by: Michael Russo <merusso@gmail.com>
@merusso
Copy link
Contributor Author

merusso commented May 17, 2023

FYI, I'm having issues running make reviewable successfully, looking into it.

Update: I never could run make lint successfully, got a generic error "Killed". So I just ran go mod tidy to address the currently failing build job.

Signed-off-by: Michael Russo <merusso@gmail.com>
pkg/addon/addon.go Outdated Show resolved Hide resolved
pkg/addon/addon.go Outdated Show resolved Hide resolved
pkg/addon/addon.go Outdated Show resolved Hide resolved
pkg/addon/addon.go Outdated Show resolved Hide resolved
pkg/addon/addon_test.go Outdated Show resolved Hide resolved
pkg/addon/addon.go Outdated Show resolved Hide resolved
pkg/addon/addon.go Outdated Show resolved Hide resolved
pkg/addon/addon.go Outdated Show resolved Hide resolved
pkg/addon/addon.go Outdated Show resolved Hide resolved
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
pkg/addon/addon.go Show resolved Hide resolved
pkg/addon/addon.go Outdated Show resolved Hide resolved
pkg/addon/addon.go Outdated Show resolved Hide resolved
pkg/addon/addon.go Outdated Show resolved Hide resolved
Signed-off-by: Michael Russo <merusso@gmail.com>
@Somefive
Copy link
Collaborator

Somefive commented Jun 1, 2023

You can try to rebase the master branch and retest.

@Somefive Somefive added the needs-rebase Pull request needs rebase. label Jun 1, 2023
merusso added 11 commits June 4, 2023 09:20
Changes:
* implement ListAddonInfo in Registry
* add interface to aid testing of listAvailableAddons
* add tests for listAvailableAddons

Signed-off-by: Michael Russo <merusso@gmail.com>
move logic from validateAddonDependencies to
calculateDependencyVersionToInstall.

Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
Signed-off-by: Michael Russo <merusso@gmail.com>
@merusso
Copy link
Contributor Author

merusso commented Jun 4, 2023

All checks pass now. I still need to do some testing, but once that completes, I think this is "ready for review" (no longer Draft)

Update: Successfully ran manual tests

@merusso merusso marked this pull request as ready for review June 4, 2023 20:55
Copy link
Member

@chivalryq chivalryq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, great job!

@Somefive Somefive removed the needs-rebase Pull request needs rebase. label Jun 5, 2023
Signed-off-by: Michael Russo <merusso@gmail.com>
@wangyikewxgm wangyikewxgm merged commit be3b990 into kubevela:master Jun 7, 2023
21 checks passed
@merusso
Copy link
Contributor Author

merusso commented Jun 7, 2023

I'll work on updating the documentation here :)

PR here: kubevela/kubevela.github.io#1240

@merusso merusso deleted the issue-5861-dependencies-version-range branch June 7, 2023 05:13
@wonderflow
Copy link
Collaborator

@merusso great job!

@merusso
Copy link
Contributor Author

merusso commented Jun 7, 2023

Thank you all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Addon dependencies version range
9 participants