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

✨ Update Cert manager tilt module and fix tilt up for CAPM3 #290

Merged
merged 2 commits into from
Oct 19, 2021
Merged

✨ Update Cert manager tilt module and fix tilt up for CAPM3 #290

merged 2 commits into from
Oct 19, 2021

Conversation

furkatgofurov7
Copy link
Member

@furkatgofurov7 furkatgofurov7 commented Sep 9, 2021 •
edited
Loading

What this PR does / why we need it:
Updates cert manager tilt module and checks if those changes affect tilt up process.
Note: to be able to successfully run CAPM3 controller dummy BMH creation has been added to tilt-up make file target.

Follows CAPI#4680 on cert manager update

@furkatgofurov7 furkatgofurov7 changed the title 🌱WIP: Update Cert manager tilt module 🌱 WIP:Update Cert manager tilt module Sep 9, 2021
@metal3-io-bot metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 9, 2021
@furkatgofurov7 furkatgofurov7 force-pushed the uplift-cert-manager/furkat branch from 6d0c97a to 48211c7 Compare September 9, 2021 20:15
@furkatgofurov7
Copy link
Member Author

/test-integration

@metal3-io-bot metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label Sep 29, 2021
@furkatgofurov7 furkatgofurov7 force-pushed the uplift-cert-manager/furkat branch from 48211c7 to bac8e5a Compare October 4, 2021 08:22
@metal3-io-bot metal3-io-bot removed the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label Oct 4, 2021
@furkatgofurov7 furkatgofurov7 force-pushed the uplift-cert-manager/furkat branch 2 times, most recently from c7dd4fb to 6cce356 Compare October 14, 2021 23:18
@metal3-io-bot metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 14, 2021
@furkatgofurov7 furkatgofurov7 force-pushed the uplift-cert-manager/furkat branch 3 times, most recently from 54eb546 to b1476c7 Compare October 14, 2021 23:29
@furkatgofurov7
Copy link
Member Author

/retitle ✨ Update Cert manager tilt module and fix tilt up for CAPM3
/test-integration

@metal3-io-bot metal3-io-bot changed the title 🌱 WIP:Update Cert manager tilt module ✨ Update Cert manager tilt module and fix tilt up for CAPM3 Oct 14, 2021
@metal3-io-bot metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 14, 2021
@furkatgofurov7
Copy link
Member Author

Local Tilt env's look after above changes:
tilt-up

@furkatgofurov7
Copy link
Member Author

/test unit

1 similar comment
@furkatgofurov7
Copy link
Member Author

/test unit

@furkatgofurov7
Copy link
Member Author

/test-integration

@furkatgofurov7
Copy link
Member Author

/assign @kashifest
/cc @Xenwar

@metal3-io-bot metal3-io-bot requested a review from Xenwar October 15, 2021 09:07
@Xenwar
Copy link
Member

Xenwar commented Oct 15, 2021

/approve

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 15, 2021
@kashifest kashifest changed the base branch from master to main October 15, 2021 13:32
@kashifest
Copy link
Member

/hold
Until prow and metal3-dev-env is tracking the main branch for CAPM3 in integration tests

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 15, 2021
@furkatgofurov7
Copy link
Member Author

/test all
/test integration

@furkatgofurov7 furkatgofurov7 force-pushed the uplift-cert-manager/furkat branch from b1476c7 to 6a9be75 Compare October 15, 2021 16:37
@furkatgofurov7
Copy link
Member Author

/unhold main branch transition landed and m3-dev-env is setup
/test-integration

@furkatgofurov7
Copy link
Member Author

/unhold
main branch transition landed and m3-dev-env is setup

@metal3-io-bot metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 15, 2021
@furkatgofurov7
Copy link
Member Author

/cc @fmuyassarov

fmuyassarov
fmuyassarov reviewed Oct 19, 2021
View reviewed changes
Copy link
Member

@fmuyassarov fmuyassarov left a comment

Choose a reason for hiding this comment

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

Looks good. What about

cluster-api-provider-metal3/Tiltfile

Line 302 in 6a9be75

union_enable_providers = {k: "" for k in user_enable_providers + always_enable_providers}.keys()
line?

@furkatgofurov7 furkatgofurov7 force-pushed the uplift-cert-manager/furkat branch from 6a9be75 to ce1230e Compare October 19, 2021 09:08
@furkatgofurov7
Copy link
Member Author

furkatgofurov7 commented Oct 19, 2021 •
edited
Loading

Looks good. What about

cluster-api-provider-metal3/Tiltfile

Line 302 in 6a9be75

union_enable_providers = {k: "" for k in user_enable_providers + always_enable_providers}.keys()

line?

It is needed to start the controllers, otherwise it will fail with:

`Loading Tiltfile at: /home/fgofurov/Projects/cluster-api-provider-metal3/Tiltfile
Installing cert-manager
Waiting for cert-manager to start
Testing cert-manager
local: curl -sSL https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/cluster-api-components.yaml | ./hack/tools/bin/envsubst | kubectl apply -f -
local: make hack/tools/bin/kustomize hack/tools/bin/envsubst-drone
 → make[1]: Entering directory '/home/fgofurov/Projects/cluster-api-provider-metal3'
 → cd hack/tools; ./install_kustomize.sh
 → Verifying kustomize version
 → make[1]: 'hack/tools/bin/envsubst-drone' is up to date.
 → make[1]: Leaving directory '/home/fgofurov/Projects/cluster-api-provider-metal3'
Successfully loaded Tiltfile (1m41.538068891s)
No resources found. Check out https://docs.tilt.dev/tutorial.html to get started!`

Looks good. What about

cluster-api-provider-metal3/Tiltfile

Line 302 in 6a9be75

union_enable_providers = {k: "" for k in user_enable_providers + always_enable_providers}.keys()

line?

So, that is needed, otherwise controllers will not be deployed (tried locally).

@furkatgofurov7
Copy link
Member Author

/test-integration

namnx228
namnx228 reviewed Oct 19, 2021
View reviewed changes
Copy link
Member

@namnx228 namnx228 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just one small nits.

Makefile Outdated Show resolved Hide resolved
@furkatgofurov7 furkatgofurov7 force-pushed the uplift-cert-manager/furkat branch from ce1230e to 2c7a715 Compare October 19, 2021 09:28
namnx228
namnx228 reviewed Oct 19, 2021
View reviewed changes
Copy link
Member

@namnx228 namnx228 left a comment

Choose a reason for hiding this comment

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

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2021
Xenwar
Xenwar reviewed Oct 19, 2021
View reviewed changes
tilt_modules/cert_manager/Tiltfile Outdated Show resolved Hide resolved
tilt_modules/cert_manager/Tiltfile Outdated Show resolved Hide resolved
tilt_modules/cert_manager/Tiltfile Outdated Show resolved Hide resolved
@metal3-io-bot metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2021
@Xenwar
Copy link
Member

Xenwar commented Oct 19, 2021

/approve

@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Xenwar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@furkatgofurov7
Copy link
Member Author

/test unit
/test-integration

@namnx228
Copy link
Member

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2021
@metal3-io-bot metal3-io-bot merged commit 0b77f3f into metal3-io:main Oct 19, 2021
@furkatgofurov7 furkatgofurov7 deleted the uplift-cert-manager/furkat branch September 23, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmuyassarov fmuyassarov fmuyassarov left review comments

@Xenwar Xenwar Xenwar left review comments

@namnx228 namnx228 namnx228 left review comments

Assignees

@kashifest kashifest

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@furkatgofurov7 @Xenwar @kashifest @metal3-io-bot @namnx228 @fmuyassarov