Skip to content

remove mounts.conf again, but patch buildah change needed to make transient mounts work for us#241

Closed
gabemontero wants to merge 2 commits intoopenshift:release-4.7from
gabemontero:fix-host-entitlements-on-openshift-buildah-level-47
Closed

remove mounts.conf again, but patch buildah change needed to make transient mounts work for us#241
gabemontero wants to merge 2 commits intoopenshift:release-4.7from
gabemontero:fix-host-entitlements-on-openshift-buildah-level-47

Conversation

@gabemontero
Copy link
Copy Markdown
Contributor

The 4.7 version of #239
that vendor's in buildah's https://github.com/containers/buildah/pull/3117/commits that allows us to use transient mounts vs. mounts.conf so we don't get those benign warning messages from buildah when the entitlement file are unavailable on RHCOS

this would ultimately replace #240 in 4.7

an options to consider, if we trust the manual verification of the buildah fix I did last week, or stop the merge of #240 in 4.7 and just go with this.

@bparees @nalind @adambkaplan

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@gabemontero: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

Fix host entitlements on openshift buildah level 47

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.

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gabemontero
To complete the pull request process, please assign bparees after the PR has been reviewed.
You can assign the PR to them by writing /assign @bparees in a comment when ready.

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

@gabemontero: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

remove mounts.conf again, but patch buildah change needed to make transient mounts work for us

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.

@gabemontero gabemontero changed the title Fix host entitlements on openshift buildah level 47 remove mounts.conf again, but patch buildah change needed to make transient mounts work for us Apr 4, 2021
@gabemontero
Copy link
Copy Markdown
Contributor Author

e2e-aws failure in kubelet terminates kube-apiserver gracefully

/test e2e-aws

@bparees
Copy link
Copy Markdown
Contributor

bparees commented Apr 5, 2021

i certainly like this fix better if we're confident in it.

@gabemontero
Copy link
Copy Markdown
Contributor Author

gabemontero commented Apr 5, 2021

OK we discussed this situation with @adambkaplan back from PTO and we've decided to do this approach separately from #240. We'll pursue merging that one, verifying separately, backporting to 4.6, and getting out in the 4.7.z stream, where if this PR goes out in a subsequent 4.7.z release, so be it. I'll open a new BZ for driving this in starting with #239 in 4.8.

We are going to try to pursue with patch manager merging 4.7 and 4.6 concurrently to expedite delivery with customers.

I've updated the doc update / release note section in the BZ about ignoring the benign warning section with this form.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 11, 2021
@gabemontero gabemontero force-pushed the fix-host-entitlements-on-openshift-buildah-level-47 branch from 4341e73 to eadc193 Compare April 11, 2021 16:00
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 11, 2021
@gabemontero
Copy link
Copy Markdown
Contributor Author

/test e2e-aws

@adambkaplan
Copy link
Copy Markdown
Contributor

@gabemontero do we have a pressing need to backport #239 to 4.7? If we've fixed the root issue already, then I'm not inclined to backport something that isn't user-facing.

@gabemontero
Copy link
Copy Markdown
Contributor Author

@gabemontero do we have a pressing need to backport #239 to 4.7? If we've fixed the root issue already, then I'm not inclined to backport something that isn't user-facing.

One could claim it is user facing @adambkaplan because if the user looks at a build log and sees those Warning messages, they may get concerned. And unless they think of looking at the release notes to confirm those warning messages are benign, perhaps they instead file a customer to case to ask or complain about them.

@bparees also wanted us to move toward this "cleaner" form long term.

WDYT?

If you are swayed, I'll open BZs for the 4.8 branch and this one, and "officially" start things.

thanks

@gabemontero
Copy link
Copy Markdown
Contributor Author

per discussion with @adambkaplan we'll do #239 in master/4.8 but as a rule we do not bump buildah like this in the z stream so close this one out ... we'll go with the openshift/builder only form that has the benign warning messages when running on rhcos

@gabemontero gabemontero deleted the fix-host-entitlements-on-openshift-buildah-level-47 branch April 19, 2021 02:11
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.

4 participants