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

Install build strategies via operator #171

Merged

Conversation

adambkaplan
Copy link
Member

@adambkaplan adambkaplan commented Nov 3, 2023

Changes

Enhance the operator to install a set of sample cluster build strategies. These were hand-copied from shipwright-io/build samples, only including ClusterBuildStrategy objects.

Fixes #155

/kind feature

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

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

Operator installs sample cluster build strategies for tools such as kaniko, buildah, and cloud-native buildpacks.

@openshift-ci openshift-ci bot added release-note do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Nov 3, 2023
@openshift-ci openshift-ci bot requested review from coreydaley and otaviof November 3, 2023 13:49
@pull-request-size pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 3, 2023
if !crdExists {
return true, nil
}
// Apply the provided manifest containing the build strategies
Copy link
Contributor

@jkhelil jkhelil Nov 6, 2023

Choose a reason for hiding this comment

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

After checkiing crds, shouldnt we wait also for the webhook svc to be up, otherwise we will face a connection refused error when we apply the stragetgies
I have seen this issue happening when the webhook is not up, and with parallel controllers, now that the logic is after the webhook install, we may not have the issue

conversion webhook for shipwright.io/v1beta1, Kind=ClusterBuildStrategy failed: Post \"https://shp-build-webhook.shipwright-operator.svc:443/convert?timeout=30s\": dial tcp 10.96.125.48:443: connect: connection refused

Copy link
Member Author

Choose a reason for hiding this comment

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

Great point! I think this explains a lot, especially with the tests driven by EnvTest. Unlike a real cluster, we have no pods being deployed for the webhook (or service, for that matter). I'll switch to using the v1alpha1 samples to see if that fixes the issue by avoiding the conversion webhook invocation.

Add shipwright-io/build v0.12.0 as a dependency, with additional
updates via `go get`:

- Update ginkgo to v2.13.0
- Update k8s.io/* to v0.26.9
@adambkaplan adambkaplan force-pushed the install-build-strategies branch from b72656a to 5260204 Compare November 6, 2023 21:01
Install the build strategy samples from shipwright-io/build, at the
v0.12.0 tag. Only cluster build strategies were added - namespace scoped
build strategies were dropped. Red Hat-specific strategies were also
dropped, in part to avoid potential trademark issues. The build strategies
use v1alpha1 APIs because this is the stored version in v0.12.0. Testing
revealed that if we reconciled v1beta1 APIs, we would need to wait for
(and mock out!) the conversion webhook deployment. Lastly, the operator
was granted RBAC permission to administer all ClusterBuildStrategies.

Reconciling build strategies required adding logic to wait for the
required CRDs to be installed on the cluster first. If a requeue is
required, the operator will report the Ready status condition as "Unknown."
The manifestival library code also had to enhanced to optionally recurse
a directory for manifests to deploy.

Finally, development of this feature revealed refactoring opportunities
with respect to our use of k8s and controller-runtime client libraries, and
the way we are organizing/managing Manifestival-driven reconcilers. Some
refactoring of test code was included to simplify future testing efforts.
@adambkaplan adambkaplan force-pushed the install-build-strategies branch from 5260204 to d423d71 Compare November 6, 2023 21:51
@adambkaplan adambkaplan changed the title WIP - Install build strategies via operator Install build strategies via operator Nov 6, 2023
@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 Nov 6, 2023
@adambkaplan
Copy link
Member Author

/assign @jkhelil

Thanks for pointing me in the right direction wrt. the conversion webhook. Reconciling with the "stored" api version bypasses the need to wait for the conversion webhook to be ready.

@jkhelil
Copy link
Contributor

jkhelil commented Nov 7, 2023

/approve

Copy link
Contributor

openshift-ci bot commented Nov 7, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jkhelil

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 Nov 7, 2023
@jkhelil
Copy link
Contributor

jkhelil commented Nov 7, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 7, 2023
@openshift-merge-bot openshift-merge-bot bot merged commit bed9d49 into shipwright-io:main Nov 7, 2023
3 checks passed
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 size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FEATURE] Install Cluster Build Strategies via Operator
2 participants