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

deploy shipwright triggers via operator #129

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jkhelil
Copy link
Contributor

@jkhelil jkhelil commented May 16, 2023

Changes

  • Add triggers.yaml to kodata
  • Deploy Shipwright Triggers alongside Shipwright Build

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

Deploy Shipwright Triggers manifest via Shipwright Operator

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 16, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign otaviof for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@jkhelil jkhelil force-pushed the deploy_triggers branch 2 times, most recently from 4beaff9 to ce5f467 Compare May 16, 2023 17:48

r.Manifest, err = manifestival.NewManifest(
buildManifest,
dataPath,

Choose a reason for hiding this comment

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

I think the trigger is the option component, so it would be great if do not deploy trigger by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is now optional, regarding if we set an env var to install the triggers or not

serviceAccountName: shipwright-triggers
containers:
- name: shipwright-triggers
image: "quay.io/jkhelil/openshift-builds-triggers:latest"

Choose a reason for hiding this comment

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

Use official image from shipwright.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the official image is not publsihed, lets wait for it to be published then, I will update it

Copy link

@liangyuanpeng liangyuanpeng left a comment

Choose a reason for hiding this comment

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

LGTM for deploy triggers via operator.

/cc @SaschaSchwarze0

Copy link
Member

@adambkaplan adambkaplan left a comment

Choose a reason for hiding this comment

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

/hold

We don't have a release for the Triggers project yet. I would rather us have a release there first (even if it is a "release candidate") before adding it to the operator.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants