Skip to content

Make: move the end-to-end tests from Bazel to Make with the new commands "make e2e-setup" and "make e2e"#4914

Merged
jetstack-bot merged 26 commits intocert-manager:masterfrom
maelvls:install.mk
Mar 17, 2022
Merged

Make: move the end-to-end tests from Bazel to Make with the new commands "make e2e-setup" and "make e2e"#4914
jetstack-bot merged 26 commits intocert-manager:masterfrom
maelvls:install.mk

Conversation

@maelvls
Copy link
Copy Markdown
Member

@maelvls maelvls commented Mar 3, 2022 •
edited
Loading

This PR is part of the effort 'moving away from Bazel' tracked in #4712

To report issues with the new make flow, feel free to reach out on the Kubernetes Slack, channel #cert-manager-dev.

To test this PR, you don't need any Kind cluster. Run this:

make -j e2e-setup

Take a look a the beautiful help:

make

Run the end-to-end tests:

make e2e

For the "fresh" first run, it takes a bit less than 1 minute and 15 seconds to create the kind cluster, download everything, and install cert-manager and the addons:

$ rm -rf bin
$ make -j e2e-setup
make -j e2e-setup  54.47s user 14.15s system 61% cpu 1:15.25 total

Re-building cert-manager and re-installing all the addons takes 28 seconds:

$ make clean
$ make -j e2e-setup
make -j e2e-setup  13.30s user 3.24s system 58% cpu 28.234 total

Re-creating the whole cluster is faster after the first time since bin/ contains all the cached images. This is more true on slow connections (in my case, the connection is not the limiting factor).

⚠️ I have decided to replace the root Makefile with what used to be make/Makefile. This won't break any existing Bazel job: the root Makefile's targets weren't used anywhere. ⚠️

/kind feature

Build: add make targets for running the end-to-end tests, as part of the Bazel replacement.

@jetstack-bot jetstack-bot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/testing Issues relating to testing approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 3, 2022
@maelvls maelvls force-pushed the install.mk branch 5 times, most recently from be1931e to 29dfd5d Compare March 3, 2022 20:13
maelvls
maelvls commented Mar 3, 2022
View reviewed changes
@maelvls maelvls force-pushed the install.mk branch 4 times, most recently from 9adc5ba to a3ae6bb Compare March 4, 2022 13:08
@maelvls
Copy link
Copy Markdown
Member Author

maelvls commented Mar 6, 2022

Yes!!!

Screenshot 2022-03-06 at 15 29 25

@jetstack-bot jetstack-bot added area/acme Indicates a PR directly modifies the ACME Issuer code area/acme/dns01 Indicates a PR modifies ACME DNS01 provider code area/acme/http01 Indicates a PR modifies ACME HTTP01 provider code area/api Indicates a PR directly modifies the 'pkg/apis' directory area/ca Indicates a PR directly modifies the CA Issuer code area/deploy Indicates a PR modifies deployment configuration area/monitoring Indicates a PR or issue relates to monitoring area/vault Indicates a PR directly modifies the Vault Issuer code labels Mar 7, 2022
@maelvls
Copy link
Copy Markdown
Member Author

maelvls commented Mar 7, 2022 •
edited
Loading

I went a bit further and I reworked the help. I also moved make/Makefile to Makefile since Makefile is useless at this point.

I know I should split in multiple PRs, but I haven't had the time to get around to that. 😥

@maelvls maelvls mentioned this pull request Mar 7, 2022
Merged
@maelvls maelvls force-pushed the install.mk branch 2 times, most recently from cb23ebb to fce22e5 Compare March 7, 2022 18:12
SgtCoDFish
SgtCoDFish approved these changes Mar 16, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@SgtCoDFish SgtCoDFish left a comment

Choose a reason for hiding this comment

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

/lgtm

Happy to merge this, thank you for doing awesome work on it 😁

@jetstack-bot jetstack-bot added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels Mar 16, 2022
@maelvls
Copy link
Copy Markdown
Member Author

maelvls commented Mar 16, 2022

/test pull-cert-manager-make-test
/test pull-cert-manager-make-e2e-v1-23

SgtCoDFish
SgtCoDFish approved these changes Mar 16, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@SgtCoDFish SgtCoDFish left a comment

Choose a reason for hiding this comment

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

/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 16, 2022
@jetstack-bot
Copy link
Copy Markdown
Contributor

jetstack-bot commented Mar 16, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maelvls, SgtCoDFish

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

The pull request process is described here

Details 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

@maelvls
Copy link
Copy Markdown
Member Author

maelvls commented Mar 16, 2022 •
edited
Loading

/test pull-cert-manager-make-e2e-v1-23

Flaky Gateway tests 😭

@maelvls
Copy link
Copy Markdown
Member Author

maelvls commented Mar 16, 2022

/test pull-cert-manager-make-e2e-v1-23
/test pull-cert-manager-make-test

@jetstack-bot jetstack-bot added dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. and removed lgtm Indicates that a PR is ready to be merged. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. labels Mar 16, 2022
jahrlin and others added 2 commits March 16, 2022 17:51
@jahrlin @maelvls
make: e2e-setup-haproxyingress: add haproxyingress arm64 image
ff2d82f 
Signed-off-by: Joakim Ahrlin <joakim.ahrlin@gmail.com>
@maelvls
make: warn people about e2e-setup-traefik and e2e-setup-haproxyingress
d30f472 
These two targets are not used by our end-to-end tests meaning that they
may either be broken or removed in the future.

Signed-off-by: Maël Valais <mael@vls.dev>
@maelvls
Copy link
Copy Markdown
Member Author

maelvls commented Mar 16, 2022

/test pull-cert-manager-make-e2e-v1-23
/test pull-cert-manager-make-test

@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Mar 16, 2022
@maelvls
Copy link
Copy Markdown
Member Author

maelvls commented Mar 16, 2022

/retest pull-cert-manager-make-e2e-v1-23

@SgtCoDFish
Copy link
Copy Markdown
Member

SgtCoDFish commented Mar 17, 2022

/lgtm

Thank you for your persistence on this!

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2022
@jetstack-bot jetstack-bot merged commit af47ae4 into cert-manager:master Mar 17, 2022
@jetstack-bot jetstack-bot added this to the v1.8 milestone Mar 17, 2022
@maelvls maelvls deleted the install.mk branch March 17, 2022 09:05
@cert-manager cert-manager deleted a comment from jetstack-bot Mar 17, 2022
This was referenced Mar 17, 2022
Merged
Closed
@jetstack-bot jetstack-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SgtCoDFish SgtCoDFish SgtCoDFish approved these changes

Assignees

@SgtCoDFish SgtCoDFish

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/acme/dns01 Indicates a PR modifies ACME DNS01 provider code area/acme/http01 Indicates a PR modifies ACME HTTP01 provider code area/acme Indicates a PR directly modifies the ACME Issuer code area/api Indicates a PR directly modifies the 'pkg/apis' directory area/ca Indicates a PR directly modifies the CA Issuer code area/deploy Indicates a PR modifies deployment configuration area/monitoring Indicates a PR or issue relates to monitoring area/testing Issues relating to testing area/vault Indicates a PR directly modifies the Vault Issuer code dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

v1.8

Development

Successfully merging this pull request may close these issues.

4 participants

@maelvls @SgtCoDFish @jetstack-bot @jahrlin