Skip to content

Remove existing Secret files in (deprovision) Pod#2430

Closed
2uasimojo wants to merge 1 commit intoopenshift:mce-2.4from
2uasimojo:HIVE-2604/mce-2.4
Closed

Remove existing Secret files in (deprovision) Pod#2430
2uasimojo wants to merge 1 commit intoopenshift:mce-2.4from
2uasimojo:HIVE-2604/mce-2.4

Conversation

@2uasimojo
Copy link
Member

@2uasimojo 2uasimojo commented Aug 27, 2024

For Pods with restartPolicy: OnFailure, a failed container may be rerun in the same Pod, which will reuse the same file system as the initial run. When we project Secrets (for credentials, certs, etc) to directories in such containers, those writes can fail the second time around because the file already exists. Fix by removing the file, if it exists, before we write it.

Note that at the time of this commit, this only affects deprovision pods:

  • imageset pods don't use ProjectToDir
  • provision pods have restartPolicy: Never

HIVE-2604

(cherry picked from commit 4af190c)

Conflicts:
contrib/pkg/utils/generic.go

For Pods with `restartPolicy: OnFailure`, a failed container may be
rerun in the same Pod, which will reuse the same file system as the
initial run. When we project Secrets (for credentials, certs, etc) to
directories in such containers, those writes can fail the second time
around because the file already exists. Fix by removing the file, if it
exists, before we write it.

Note that at the time of this commit, this only affects deprovision
pods:
- imageset pods don't use ProjectToDir
- provision pods have `restartPolicy: Never`

HIVE-2604

(cherry picked from commit 4af190c)

Conflicts:
	contrib/pkg/utils/generic.go
@openshift-ci openshift-ci bot requested review from dlom and lleshchi August 27, 2024 17:22
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 27, 2024
@2uasimojo
Copy link
Member Author

/assign @dlom

Manual backport, just had to get around a var name change.

@codecov
Copy link

codecov bot commented Aug 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 57.58%. Comparing base (47bf8a8) to head (9e91958).
Report is 5 commits behind head on mce-2.4.

Files Patch % Lines
contrib/pkg/utils/generic.go 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           mce-2.4    #2430      +/-   ##
===========================================
- Coverage    57.59%   57.58%   -0.01%     
===========================================
  Files          187      187              
  Lines        25853    25855       +2     
===========================================
  Hits         14889    14889              
- Misses        9715     9717       +2     
  Partials      1249     1249              
Files Coverage Δ
contrib/pkg/utils/generic.go 10.61% <0.00%> (-0.20%) ⬇️

@dlom
Copy link
Contributor

dlom commented Aug 27, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2024
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 27, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, dlom

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-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 47bf8a8 and 2 for PR HEAD 9e91958 in total

@2uasimojo
Copy link
Member Author

/override ci/prow/security

Backport of #2387, if that happens.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 27, 2024

@2uasimojo: Overrode contexts on behalf of 2uasimojo: ci/prow/security

Details

In response to this:

/override ci/prow/security

Backport of #2387, if that happens.

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-sigs/prow repository.

@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

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-sigs/prow repository.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 27, 2024
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 27, 2024

@2uasimojo: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/verify 9e91958 link unknown /test verify
ci/prow/security 9e91958 link unknown /test security
ci/prow/coverage 9e91958 link unknown /test coverage
ci/prow/images 9e91958 link unknown /test images
ci/prow/e2e-pool 9e91958 link unknown /test e2e-pool
ci/prow/e2e 9e91958 link unknown /test e2e
ci/prow/unit 9e91958 link unknown /test unit

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-sigs/prow repository. I understand the commands that are listed here.

@2uasimojo
Copy link
Member Author

Closing in favor of successful bot cherry-pick via #2431 since #2400 landed.

@2uasimojo 2uasimojo closed this Aug 27, 2024
@2uasimojo 2uasimojo deleted the HIVE-2604/mce-2.4 branch August 27, 2024 21:28
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. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants