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.

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

Update release process for webhook certificate #1384

Merged

Conversation

SaschaSchwarze0
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 commented Sep 17, 2023

Changes

Part of #1344

I am changing make generate to also run the hack/patch-crds-with-conversion.sh script so that the conversion block is now part of the CRDs that are committed.

With that, we are producing an incomplete release yaml where two things are missing: the shipwright-build-webhook-cert secret and the caBundle in the CRDs.

I am introducing hack/setup-webhook-cert.sh which can run against a cluster to create or update the webhook cert and configure the caBundle.

I am changing make install-controller-kind to call this script so that it can be used again as single make target to setup Shipwright Build. That is also required for the setup GitHub action which calls this make target and nothing else. The prepare-conversion target is therefore obsolete.

I am changing the nightly build to publish a latest.txt containing the timestamp. I am using this in the README to provide an easy command to install the latest which includes invoking the setup-webhook-cert.sh script.

I tested as much as I could locally, but will need to retest this after the nightly build ran for the first time with this.

Users who will install v0.12 will need to apply the release YAML and then run the setup script from the v0.12 tag. We will mention this in the release notes and will need to update the README again once we release.

The changes to release.sh are non-functional. I only added an env var to override the platform from the outside (which makes testing much faster if you call that script and just build for one platform instead of all), and adjusted the ko commands to be consistent by always using the long parameter name. And I needed to add a dummy KO_DOCKER_REPO to the sample build strategy command - it does not actually build any binary, but for me it always failed with exit code 123 (or 132 ?) until I specified it.

Finally, I had to fix the integration tests. Basically, whenever Kubernetes performs an operation on Shipwright artifacts, it will perform it on the Beta object. This applies to deletion propagation mainly: when a Build owns all BuildRuns and the Build is deleted, or when a namespace is deleted that contains Shipwright artifacts. Because of this, I changed the integration test to setup the CRDs with a URL with host.docker.internal, and start the webhook in the BeforeSuite of the integration tests. This works locally, but not in the GitHub action. The tests are still green because I reduced the wait time for the namespace deletion and when it is not deleted, it only prints a warning, and this warning (failed to delete namespace: test-build-291, with error: timed out waiting for the condition) happens for every test run. We should eventually address this.

Submitter Checklist

  • Includes tests if functionality changed/was added
  • Includes docs if changes are user-facing
  • Set a kind label on this PR
  • Release notes block has been filled in, or marked NONE

Release Notes

Installing a nightly release now requires you to run a post-script that sets up the TLS certificate of the conversion webhook

@SaschaSchwarze0 SaschaSchwarze0 added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 17, 2023
@SaschaSchwarze0 SaschaSchwarze0 added this to the release-v0.12.0 milestone Sep 17, 2023
@openshift-ci openshift-ci bot added the release-note Label for when a PR has specified a release note label Sep 17, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 17, 2023
@SaschaSchwarze0 SaschaSchwarze0 marked this pull request as draft September 21, 2023 19:24
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 21, 2023
@SaschaSchwarze0 SaschaSchwarze0 changed the title Update nightly build to cover caBundle setup for webhook Update release process for webhook certificate Sep 21, 2023
@SaschaSchwarze0 SaschaSchwarze0 marked this pull request as ready for review September 21, 2023 19:49
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 21, 2023
@openshift-ci openshift-ci bot requested a review from otaviof September 21, 2023 19:49
@SaschaSchwarze0 SaschaSchwarze0 force-pushed the sascha-cabundle branch 3 times, most recently from bd3f915 to 0dd6ab0 Compare September 21, 2023 19:57
@qu1queee qu1queee removed the request for review from otaviof September 22, 2023 06:44
@SaschaSchwarze0
Copy link
Member Author

Seemingly I broke in the integration tests, will check this later today.

Copy link
Contributor

@qu1queee qu1queee left a comment

Choose a reason for hiding this comment

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

great work

Left one comment, overall looks neat!

.github/workflows/release.yaml Outdated Show resolved Hide resolved
@SaschaSchwarze0 SaschaSchwarze0 force-pushed the sascha-cabundle branch 9 times, most recently from a28d28b to 8620a2c Compare September 23, 2023 14:36
Copy link
Contributor

@qu1queee qu1queee left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 26, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qu1queee

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 26, 2023
@openshift-merge-robot openshift-merge-robot merged commit 0d965b4 into shipwright-io:main Sep 26, 2023
12 checks passed
@SaschaSchwarze0 SaschaSchwarze0 deleted the sascha-cabundle branch August 7, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Label for when a PR has specified a release note size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants