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

Feat/shipwright strategies #601

Merged
merged 11 commits into from
Jan 21, 2025
Merged

Conversation

sgaist
Copy link
Collaborator

@sgaist sgaist commented Jan 8, 2025

Describe your changes

This PR implements the Shipwright CRDs to use paketo buildpacks to build the final image.

It uses the creator tool from the buildpack's lifecycle.

There's a test validating the build and use of the strategy.

The full builder is currently used so that the strategy can cover a larger set of projects.

sgaist added 8 commits January 8, 2025 10:24
This command replaces the bash scrip that triggers the various
lifecycle steps done by kpack so that it can be used with
Shipwright.
Now the output of the various commands used are forwarded
to the helper output so there's better information available
to the user.
It can still be triggered using the script.
This simplifies starting the development environment.
@sgaist sgaist requested a review from a team as a code owner January 8, 2025 16:51
The lifecycle repository provides the creator binary which
runs each step of the build lifecycle in a similar fashion as
what the original script did as well as its go counterpart.

This commit thus removes all of that and updates the manifest
to use creator. Python tests have been added to ensure that
the buildrun is successfull.
@sgaist sgaist force-pushed the feat/shipwright-strategies branch from 78cfdc2 to 83cd71d Compare January 20, 2025 15:40
Copy link
Member

@olevski olevski left a comment

Choose a reason for hiding this comment

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

A general question. I am only looking at a diff of this PR. But is the idea that the kubectl CLI will be called in production?

Also it is better to use a BuildStrategy rather than a ClusterBuildStrategy. Since the cluster one is cluster scoped and not namespaced. IMO it is better to have it namespaced - i.e. safer and it requries less permissions in the cluster.

.devcontainer/devcontainer.json Outdated Show resolved Hide resolved
It's not needed anymore.
@sgaist
Copy link
Collaborator Author

sgaist commented Jan 21, 2025

A general question. I am only looking at a diff of this PR. But is the idea that the kubectl CLI will be called in production?

AFAIK, no. I wrote the test to confirm that things are working as intended with the current state of development.
There's an update to the API in the work that will handle the shipwright resources creation beside the strategy so this test should be replaced with the new one.

Also it is better to use a BuildStrategy rather than a ClusterBuildStrategy. Since the cluster one is cluster scoped and not namespaced. IMO it is better to have it namespaced - i.e. safer and it requires less permissions in the cluster.

Agreed, I'll push a new version using a BuildStrategy.

ClusterBuildStrategy is too broad for the use of Renku.

Make use of BuildStrategy and update tests to use a namespace.
@olevski olevski self-requested a review January 21, 2025 10:17
@sgaist sgaist merged commit 7386212 into kpack-resources Jan 21, 2025
11 of 12 checks passed
@sgaist sgaist deleted the feat/shipwright-strategies branch January 21, 2025 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants