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

Add ko build strategy sample #603

Merged
merged 1 commit into from
Feb 26, 2021
Merged

Add ko build strategy sample #603

merged 1 commit into from
Feb 26, 2021

Conversation

SaschaSchwarze0
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 commented Feb 19, 2021 •
edited
Loading

Changes

Based on this inquiry, I am adding the ko sample build strategy that I recently created. I added a big note to the documentation about the creative interpretation of the contextDir property.

Submitter Checklist

  • Includes tests if functionality changed/was added
  • Includes docs if changes are user-facing
  • Release notes block has been filled in, or marked NONE

Release Notes

- Adding a sample build strategy for [ko](https://github.com/google/ko)

@openshift-ci-robot openshift-ci-robot added the release-note Label for when a PR has specified a release note label Feb 19, 2021
@SaschaSchwarze0 SaschaSchwarze0 changed the title Add ko build strategy sample WIP Add ko build strategy sample Feb 19, 2021
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 19, 2021
@SaschaSchwarze0
Copy link
Member Author

The Travis build uses something like 172.18.0.3:5000/shipwright-io/build-e2e:ko (with more than one :) as image during e2e which breaks my logic to extract the tag name. Let me fix that. :-)

@SaschaSchwarze0 SaschaSchwarze0 force-pushed the sascha-ko-build-strategy branch from 11d4aec to 939e255 Compare February 19, 2021 09:45
@SaschaSchwarze0 SaschaSchwarze0 changed the title WIP Add ko build strategy sample Add ko build strategy sample Feb 19, 2021
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 19, 2021
zhangtbj
zhangtbj reviewed Feb 22, 2021
View reviewed changes
samples/buildrun/buildrun_ko_cr.yaml Outdated Show resolved Hide resolved
samples/buildstrategy/ko/buildstrategy_ko_cr.yaml Show resolved Hide resolved
samples/buildstrategy/ko/buildstrategy_ko_cr.yaml
name: ko
spec:
buildSteps:
- name: prepare
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that possible to use the single build-and-push step? Or do we some permission problem when building by ko with root?

Copy link
Member Author

Choose a reason for hiding this comment

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

One can probably run as root, I was just keeping it similar to the Buildpacks sample where we run the "real" logic as non-root.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep it similar to the Kaniko?

I think it is better to keep it simple as possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

@zhangtbj I think is fine to have it like this. The more strategies we have that can run non-root the better. Otherwise users will be constrained to their infra. Similar to buildah in IKS.

@SaschaSchwarze0 SaschaSchwarze0 changed the title Add ko build strategy sample WIP Add ko build strategy sample Feb 22, 2021
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 22, 2021
@SaschaSchwarze0 SaschaSchwarze0 force-pushed the sascha-ko-build-strategy branch from 939e255 to 5cb806d Compare February 22, 2021 08:08
@SaschaSchwarze0 SaschaSchwarze0 changed the title WIP Add ko build strategy sample Add ko build strategy sample Feb 22, 2021
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 22, 2021
@sbose78
Copy link
Member

sbose78 commented Feb 22, 2021

Hi @imjasonh ! Could you please review ?

@imjasonh
Copy link
Contributor

/lgtm

Thanks for adding this! 👍

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 22, 2021
@SaschaSchwarze0 SaschaSchwarze0 added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 24, 2021
@SaschaSchwarze0 SaschaSchwarze0 force-pushed the sascha-ko-build-strategy branch from 5cb806d to 05f38d4 Compare February 26, 2021 12:44
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 26, 2021
@SaschaSchwarze0
Copy link
Member Author

Updated to ko v0.8.1.

qu1queee
qu1queee approved these changes Feb 26, 2021
View reviewed changes
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

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 26, 2021
sbose78
sbose78 approved these changes Feb 26, 2021
View reviewed changes
Copy link
Member

@sbose78 sbose78 left a comment

Choose a reason for hiding this comment

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

Thank you!

samples/build/build_ko_cr.yaml
name: ko
kind: ClusterBuildStrategy
output:
image: image-registry.openshift-image-registry.svc:5000/build-examples/build-operator
Copy link
Member

Choose a reason for hiding this comment

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

.. high time we stop using openshift internal registry URLs in examples ( blame me ). Maybe in a different PR :)

@sbose78
Copy link
Member

sbose78 commented Feb 26, 2021

/lgtm
/approve

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbose78

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-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 26, 2021
@openshift-merge-robot openshift-merge-robot merged commit 965569b into shipwright-io:master Feb 26, 2021
@SaschaSchwarze0 SaschaSchwarze0 mentioned this pull request Feb 28, 2021
Merged
4 tasks
@SaschaSchwarze0 SaschaSchwarze0 deleted the sascha-ko-build-strategy branch March 3, 2021 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qu1queee qu1queee qu1queee approved these changes

@sbose78 sbose78 sbose78 approved these changes

@adambkaplan adambkaplan Awaiting requested review from adambkaplan

@zhangtbj zhangtbj Awaiting requested review from zhangtbj

Assignees

@imjasonh imjasonh

@sbose78 sbose78

@qu1queee qu1queee

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
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@SaschaSchwarze0 @sbose78 @imjasonh @openshift-ci-robot @qu1queee @zhangtbj @openshift-merge-robot