Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Add cert-manager templates to providers #1162

Merged
merged 2 commits into from
Nov 16, 2021
Merged

Add cert-manager templates to providers #1162

merged 2 commits into from
Nov 16, 2021

Conversation

saimanoj01
Copy link
Contributor

@saimanoj01 saimanoj01 commented Nov 15, 2021

What this PR does / why we need it

In CAPI v1.0.0, it moved away from relying on a packaged version of cert-manager manifests during management cluster creation. Instead it is pulling the cert-manager manifest from github. This change ensures that CAPI uses the cert-manager manifests packaged along with the providers instead of pulling from github.

Which issue(s) this PR fixes

Fixes #1114

Describe testing done for PR

  1. Deleted the ~/config/tanzu/tkg directory and successfully created a management cluster. Made sure that the cert-manager image is being pulled from the tkg registry.
  2. Created a management cluster with the config file already populated with cert-manager entry.

Release note

Package cert-manager v1.5.3 manifests along with the providers. 

PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1162/20211115162613/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1162/20211115165108/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1162/20211115170203/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@prkalle prkalle self-assigned this Nov 15, 2021
Copy link
Contributor

@prkalle prkalle left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1162/20211115205607/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

pkg/v1/tkg/tkgconfigupdater/provider.go Show resolved Hide resolved
pkg/v1/tkg/tkgconfigupdater/provider.go Outdated Show resolved Hide resolved
pkg/v1/tkg/tkgconfigupdater/provider.go Outdated Show resolved Hide resolved
@anujc25 anujc25 added kind/bug PR/Issue related to a bug ok-to-merge PRs should be labelled with this before merging labels Nov 15, 2021
Copy link
Contributor

@anujc25 anujc25 left a comment

Choose a reason for hiding this comment

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

Is the addition of pkg/v1/tkg/fakes/providers/providers.zip file intentional? Is it getting used for unit tests?

Otherwise LGTM

@saimanoj01
Copy link
Contributor Author

@anujc25 Yes, that was intentional. I modified it for the sake of unit tests. Thanks

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1162/20211116011715/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Copy link
Contributor

@imikushin imikushin left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comments! Just one small detail and it's good to go.

pkg/v1/tkg/tkgconfigupdater/provider.go Outdated Show resolved Hide resolved
Copy link
Contributor

@imikushin imikushin left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Thank you!

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1162/20211116171252/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1162/20211116171326/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1162/20211116172213/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-not-required kind/bug PR/Issue related to a bug ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Seeing errors installing cert-manager v1.5.3 after the CAPI bump
5 participants