Skip to content

Conversation

@nalind
Copy link
Member

@nalind nalind commented Jul 14, 2021

Walk back a bit of #220: don't assume that we can set up a user namespace when we're invoked as openshift-manage-dockerfile or openshift-git-clone, even when not explicitly given mappings to use, since those containers haven't traditionally been run with privileged=true.

@openshift-ci openshift-ci bot requested review from coreydaley and gabemontero July 14, 2021 14:36
…le/git-clone

Don't assume that we can set up a user namespace when we're invoked as
openshift-manage-dockerfile or openshift-git-clone, even when not
explicitly given mappings to use, since those containers haven't
traditionally been run with privileged=true.

Signed-off-by: Nalin Dahyabhai <[email protected]>
@nalind nalind force-pushed the expected-privileges branch from b3a0b06 to 70bd718 Compare July 14, 2021 14:51
@gabemontero
Copy link
Contributor

@vrutkovs FYI ... eventually, I wonder if we should create an optional OKD version of e2e-aws-builds in this repo to catch whatever problem you saw in the nightlies .... WDYT?

@nalind
Copy link
Member Author

nalind commented Jul 14, 2021

I'm unlikely to ever argue against adding tests for a thing. How, and how far, does the OKD version diverge from the existing e2e-aws-builds test, though?

@gabemontero
Copy link
Contributor

On the surface @nalind

/lgtm

now we need @adambkaplan for the approve label.

And, per the feedback from @vrutkovs I'm wondering if there is anything besides adding the equivalent of https://github.com/openshift/release/blob/master/ci-operator/jobs/openshift/cluster-samples-operator/openshift-cluster-samples-operator-master-presubmits.yaml#L505-L573 for openshift/builder so we can get some OKD validation for this.

An optional okd-e2e-aws-builds that we could launch from this PR.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2021
@gabemontero
Copy link
Contributor

I'm unlikely to ever argue against adding tests for a thing. How, and how far, does the OKD version diverge from the existing e2e-aws-builds test, though?

I'll defer to @vrutkovs ... but presumably enough for the OKD nightlies to flag something.

@vrutkovs
Copy link

How, and how far, does the OKD version diverge from the existing e2e-aws-builds test, though?

The main difference is FCOS is used in machine-os-content, so host system is Fedora.
Other changes are insignificant (community operators only, custom installer using FCOS and such).

I can create okd-specific builder test, but we're hitting a DPTP bug in 4.9 rehearsals

Copy link
Contributor

@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.

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 14, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, gabemontero, nalind

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2021
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 5cf7187 into openshift:master Jul 14, 2021
@nalind nalind deleted the expected-privileges branch July 14, 2021 18:16
@vrutkovs
Copy link

That worked - https://amd64.origin.releases.ci.openshift.org/releasestream/4.9.0-0.okd/release/4.9.0-0.okd-2021-07-14-215804, thank you!

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. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants