Skip to content
This repository was archived by the owner on Dec 1, 2022. It is now read-only.

Inject images and download manifests for S-O#1150

Merged
openshift-ci[bot] merged 1 commit into
openshift:release-v1.3from
skonto:image_injection_1.3
Jun 24, 2022
Merged

Inject images and download manifests for S-O#1150
openshift-ci[bot] merged 1 commit into
openshift:release-v1.3from
skonto:image_injection_1.3

Conversation

@skonto
Copy link
Copy Markdown

@skonto skonto commented Jun 20, 2022

@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 Jun 20, 2022
@openshift-ci openshift-ci Bot requested review from alanfx and mgencur June 20, 2022 07:56
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 20, 2022
@skonto
Copy link
Copy Markdown
Author

skonto commented Jun 20, 2022

Tests fail due to permissions issues when running make generated-files

go: downloading knative.dev/hack v0.0.0-20220224013837-e1785985d364
github.com/openshift-knative/serverless-operator/hack imports
	knative.dev/hack: mkdir /go/pkg/mod/cache/download/knative.dev: permission denied
github.com/openshift-knative/serverless-operator/hack imports

Will change GOPATH to /tmp.

@skonto skonto force-pushed the image_injection_1.3 branch from c72a97f to ef3bcc9 Compare June 20, 2022 10:09
@skonto skonto force-pushed the image_injection_1.3 branch from ef3bcc9 to f2e3875 Compare June 20, 2022 11:02
@skonto
Copy link
Copy Markdown
Author

skonto commented Jun 20, 2022

/retest

1 similar comment
@skonto
Copy link
Copy Markdown
Author

skonto commented Jun 20, 2022

/retest

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jun 20, 2022

@skonto: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@skonto skonto changed the title [wip] inject images and download manifests for S-O Inject images and download manifests for S-O Jun 22, 2022
@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 Jun 22, 2022
@skonto skonto requested a review from nak3 June 22, 2022 10:03
@skonto
Copy link
Copy Markdown
Author

skonto commented Jun 22, 2022

/hold

@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 Jun 22, 2022
Comment thread openshift/e2e-common.sh
local SERVERLESS_DIR=$(mktemp -d)
local CURRENT_DIR=$(pwd)
git clone --depth 1 https://github.com/openshift-knative/serverless-operator.git ${SERVERLESS_DIR}
git clone --branch inject_serving_images --depth 1 https://github.com/skonto/serverless-operator.git ${SERVERLESS_DIR}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We will use main once we merge all PRs.

Copy link
Copy Markdown

@nak3 nak3 left a comment

Choose a reason for hiding this comment

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

I know you mentioned "Ingress will come as a next step" in openshift-knative/serverless-operator#1630 (comment) but could you please add the ingress into this PR?
I want to see the overview and it will not make so much difficult to review I guess.

@skonto
Copy link
Copy Markdown
Author

skonto commented Jun 24, 2022

I know you mentioned "Ingress will come as a next step" in openshift-knative/serverless-operator#1630 (comment) but could you please add the ingress into this PR? I want to see the overview and it will not make so much difficult to review I guess.

In order for ingress to be added I need to add the related release files and patches in the corresponding mid stream net-* repos.
Then I will remove them from the S-O and just download them from their midstream repo at different places like here and S-O.
Of course when a branch is created at the net-* midstream repos we will need to add those release files. In this PR it is done automatically via the download_release_artifacts.sh script during branch cut. We need something similar (I dont want to move these ingress files in this repo btw).
Regarding ingress image injection there are some env variables but not created by the ci, however we can still use them for injection.

I am not sure why this is difficult to review maybe @pierDipi can validate the approach here or we can go through the code. I didnt want to do all at once (independent small steps) but if that is the way to go I will do the rest when I get back (next week I am off).

@nak3
Copy link
Copy Markdown

nak3 commented Jun 24, 2022

/assign @pierDipi

As you mentioned, net-* images are not built by this repo so just wanted to make sure how it is used. Seeing only this PR looks no problem. @pierDipi can validate, then I would like to ask him.

@nak3
Copy link
Copy Markdown

nak3 commented Jun 24, 2022

/lgtm
/approve

As talked in the slack.

@nak3
Copy link
Copy Markdown

nak3 commented Jun 24, 2022

Please feel free to unhold.

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 24, 2022
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jun 24, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nak3, skonto

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

@skonto
Copy link
Copy Markdown
Author

skonto commented Jun 24, 2022

@nak3 would you mind issuing a PR to revert git clone --branch inject_serving_images --depth 1 https://github.com/skonto/serverless-operator.git ${SERVERLESS_DIR} once the other PR at S-O is merged?
Also part of this needs to go to main.

@skonto
Copy link
Copy Markdown
Author

skonto commented Jun 24, 2022

/unhold

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 24, 2022
@openshift-ci openshift-ci Bot merged commit dd2c4ea into openshift:release-v1.3 Jun 24, 2022
@pierDipi
Copy link
Copy Markdown
Member

This LGTM, thanks for doing this @skonto and for your review @nak3 !

openshift-ci Bot pushed a commit that referenced this pull request Jul 13, 2022
* inject images, generate manifests (#1150)

* Revert temoprary branch for image injection (#1159)

Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
openshift-merge-robot pushed a commit that referenced this pull request Jul 27, 2022
* [RELEASE-1.4] Inject images, generate manifests (#1169)

* inject images, generate manifests (#1150)

* Revert temoprary branch for image injection (#1159)

Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>

* fixes for 1.5

* Kourier image injection (#1173)

* Revert temoprary branch for image injection (#1186)

Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
openshift-merge-robot pushed a commit that referenced this pull request Aug 1, 2022
* [RELEASE-1.4] Inject images, generate manifests (#1169)

* inject images, generate manifests (#1150)

* Revert temoprary branch for image injection (#1159)

Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>

* fixes for 1.6

* Kourier image injection (#1173)

* Revert temoprary branch for image injection (#1186)

Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
openshift-merge-robot pushed a commit that referenced this pull request Aug 2, 2022
* [RELEASE-1.4] Inject images, generate manifests (#1169)

* inject images, generate manifests (#1150)

* Revert temoprary branch for image injection (#1159)

Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>

* fixes for main

* Kourier image injection (#1173)

* Revert temoprary branch for image injection (#1186)

* add pdb fix

* Revert "add pdb fix"

This reverts commit 1790632.

Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants