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

Feat(charts): add cert-manager support for "cluster-gateway" component in chart "vela-core" #3848

Merged
merged 3 commits into from
May 12, 2022
Merged

Feat(charts): add cert-manager support for "cluster-gateway" component in chart "vela-core" #3848

merged 3 commits into from
May 12, 2022

Conversation

cnfatal
Copy link
Contributor

@cnfatal cnfatal commented May 10, 2022 •
edited

Description of your changes

Fixes #3847

I have:

  • Read and followed KubeVela's contribution process.
  • Related Docs updated properly. In a new feature or configuration option, an update to the documentation is necessary.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Special notes for your reviewer

should review per commit to find out changes instead review the whole changes together.

@cnfatal cnfatal changed the title add cert-manager support for "cluster-gateway" component in chart "vela-core" Feat(charts): add cert-manager support for "cluster-gateway" component in chart "vela-core" May 10, 2022
@cnfatal
Copy link
Contributor Author

cnfatal commented May 10, 2022

Some check faild.

installing kustomize-v3.8.2 into /root/go/bin
/root/go/bin/kustomize exists. Remove it first.
make: *** [makefiles/dependency.mk:64: kustomize] Error 1
Error: Process completed with exit code 2.

It cause by kustomize install script not this PR, please check it.

https://github.com/kubernetes-sigs/kustomize/blob/c3b5d8fa195ad70d75dd884379216b99184a7d56/hack/install_kustomize.sh#L78-L84

@codecov
Copy link

codecov bot commented May 10, 2022 •
edited

Codecov Report

Merging #3848 (2923f8b) into master (0529ad8) will decrease coverage by 7.52%.
The diff coverage is n/a.

❗ Current head 2923f8b differs from pull request most recent head 89896f9. Consider uploading reports for the commit 89896f9 to get more accurate results

@@            Coverage Diff             @@
##           master    #3848      +/-   ##
==========================================
- Coverage   64.10%   56.58%   -7.53%     
==========================================
  Files         312      202     -110     
  Lines       29692    19016   -10676     
==========================================
- Hits        19035    10760    -8275     
+ Misses       8202     6942    -1260     
+ Partials     2455     1314    -1141
Flag Coverage Δ
apiserver-unittests ?
core-unittests 56.58% <ø> (-0.13%) ⬇️
e2e-multicluster-test ?
e2e-rollout-tests ?
e2etests ?

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

Impacted Files Coverage Δ
pkg/controller/utils/actions.go 0.00% <0.00%> (-80.00%) ⬇️
...v/v1alpha2/componentdefinition/mutating_handler.go 0.00% <0.00%> (-74.00%) ⬇️
pkg/resourcekeeper/containsresources.go 0.00% <0.00%> (-71.43%) ⬇️
pkg/resourcetracker/optimize.go 0.00% <0.00%> (-70.59%) ⬇️
.../manualscalertrait/manualscalertrait_controller.go 0.00% <0.00%> (-67.31%) ⬇️
pkg/oam/auxiliary.go 13.33% <0.00%> (-63.34%) ⬇️
pkg/utils/jwt.go 0.00% <0.00%> (-60.00%) ⬇️
pkg/utils/json.go 0.00% <0.00%> (-60.00%) ⬇️
pkg/resourcetracker/tree.go 14.73% <0.00%> (-58.95%) ⬇️
pkg/policy/envbinding/placement.go 35.00% <0.00%> (-55.00%) ⬇️
... and 197 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 02f5a96...89896f9. Read the comment docs.

@Somefive
Copy link
Collaborator

Seems there are some problems with the helm chart install process. Did you verify the chart by helm install kubevela ./chart/vela-core --wait --debug and see if it can be correctly installed? If so, I will try to reproduce the installation problems later.

As for the kustomize issue, we are looking into it.

@Somefive
Copy link
Collaborator

BTW, thanks for the contributing! The modification generally looks good to me. Let's work out the CI.

@wonderflow
Copy link
Collaborator

Some check faild.

installing kustomize-v3.8.2 into /root/go/bin
/root/go/bin/kustomize exists. Remove it first.
make: *** [makefiles/dependency.mk:64: kustomize] Error 1
Error: Process completed with exit code 2.

It cause by kustomize install script not this PR, please check it.

https://github.com/kubernetes-sigs/kustomize/blob/c3b5d8fa195ad70d75dd884379216b99184a7d56/hack/install_kustomize.sh#L78-L84

I think it's caused by this PR #3846

Now it's merged, you could try rebase the master and the CI should be fixed.

@cnfatal
fix(charts): remove a not built in cap "containerizedwokrload" from d…
3901f58 
…isableCaps

Signed-off-by: cnfatal <cnfatal@gmail.com>
@cnfatal
Perf(charts): mv cluster-gateway to a dir
0d45a78 
Signed-off-by: cnfatal <cnfatal@gmail.com>
@cnfatal
Feat(charts): support cert-manager on "cluster-gateway"
89896f9 
changes:
1. replace flag "--cert-dir" with "--tls-cert-file" and "--tls-private-key-file" due to cert-manager Certificate only support "k8s" style tls secret(with files "tls.crt","tls.key","ca.crt").
2. add cert-manager cainject annotation for APIService "v1alpha1.cluster.core.oam.dev"
3. add cert-manager Certificate for secret  "<release>-cluster-gateway-tls"
4. fix a typo in `values.yaml` , "optimize.optimizeCachedGvks" -> "optimize.cachedGvks" withch cause helm template failed

Signed-off-by: cnfatal <cnfatal@gmail.com>
@cnfatal
Copy link
Contributor Author

cnfatal commented May 12, 2022 •
edited

/retest

wonderflow
wonderflow approved these changes May 12, 2022
View reviewed changes
Copy link
Collaborator

@wonderflow wonderflow left a comment

Choose a reason for hiding this comment

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

thanks!

Somefive
Somefive approved these changes May 12, 2022
View reviewed changes
Copy link
Collaborator

@Somefive Somefive left a comment

Choose a reason for hiding this comment

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

Great Job!

@wonderflow wonderflow merged commit a8ad79b into kubevela:master May 12, 2022
@Somefive Somefive mentioned this pull request May 16, 2022
Closed
4 tasks
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

@Somefive Somefive Somefive approved these changes

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

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

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

Successfully merging this pull request may close these issues.

[Feature] (charts) add cert-manager support for "cluster-gateway" tls certs and APIService
3 participants
@cnfatal @Somefive @wonderflow