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.

Sign up for GitHub

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

Jump to bottom

[Addon] Fix #695 using vela existing addon packaging #714

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

[Addon] Fix #695 using vela existing addon packaging #714

wants to merge 8 commits into from

Conversation

scodeman
Copy link

@scodeman scodeman commented Aug 2, 2023

Description of your changes

As mentionned in #695 the addon chart published by hack/addons/syn_addon_package.go does not generate a proper helm chart with a Chart.yaml in it.

This pull request makes use of the existing vela cli capability to publish addons and that properly covers the generation of the Chart.yaml file as shown in vela source code github.com/oam-dev/kubevela/pkg/addon package that is using the MakeChartCompatible routine. (See https://github.com/kubevela/kubevela/blob/e109d4e525dfc253e861c82901d3947688555d7c/pkg/addon/utils.go#L367)

The PR also adds mandatory template.cue where missing and fix invalid (non semver) chart version on spark-on-k8s-operator experimental addon

Fixes #695

How has this code been tested?

The change has been tested following the github actions:
go mod tidy
cd addons && go run ../../hack/addons/syn_addon_package.go ./ https://kubevela.github.io/catalog/official
cd experimental/addons && go run ../../hack/addons/syn_addon_package.go ./ https://kubevela.github.io/catalog/experimental

Checklist

I have:

  • Title of the PR starts with type (e.g. [Addon] , [example] or [Doc]).
  • Updated/Added any relevant documentation and examples.
  • New addon should be put in experimental.
  • Update addon should modify the version in metadata.yaml to generate a new version.

@scodeman
Copy link
Author

scodeman commented Aug 2, 2023 •
edited
Loading

Regarding the failed addon-e2e test.
The changes of the following files are to make the original addon chart proper as vela expect a mandatory template.cue file
(even with empty output) for the addon.

  • addons/o11y-definitions/template.cue
  • addons/vegeta/template.cue
  • experimental/addons/canary-rollout-istio/template.cue
  • experimental/addons/envoy-gateway/template.cue
  • experimental/addons/vector-config/template.cue

The chart version seems unfortunately to be linked to the actual addon application tag in github (instead of using appVersion) so I am not sure my PR has to update the chart version, should it ?

@wangyikewxgm
Copy link
Collaborator

Thanks, please refer the failed CI info and fix them.

@scodeman
Copy link
Author

Note that I could not use semver build metadata due to kubernetes/kubernetes#115055 issue

@scodeman
Copy link
Author

scodeman commented Sep 11, 2023 •
edited
Loading

Thanks, please refer the failed CI info and fix them.

Hi @wangyikewxgm , @wonderflow, @barnettZQG, @Somefive, done, waiting for your review

@scodeman scodeman requested a review from wonderflow December 20, 2023 15:25
@wonderflow
Copy link
Collaborator

ping @Somefive @barnettZQG , please help review, this feature can be useful.

@scodeman
Copy link
Author

scodeman commented May 2, 2024

ping @Somefive @barnettZQG , @wangyikewxgm please help review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wonderflow wonderflow wonderflow approved these changes

@wangyikewxgm wangyikewxgm Awaiting requested review from wangyikewxgm wangyikewxgm is a code owner

@barnettZQG barnettZQG Awaiting requested review from barnettZQG barnettZQG is a code owner

@Somefive Somefive Awaiting requested review from Somefive Somefive is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing Chart.yaml in addon charts
3 participants
@scodeman @wangyikewxgm @wonderflow