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

osd: use cp -a command for copying init-containers binary #12501

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

Sheetalpamecha
Copy link
Contributor

remove copy-binaries cmd and related code

Description of your changes:

Which issue is resolved by this Pull Request:
Resolves #5296

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • Skip Tests for Docs: If this is only a documentation change, add the label skip-ci on the PR.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@Sheetalpamecha Sheetalpamecha changed the title osd: use cp -a command for copying init-containers osd: use cp -a command for copying init-containers binary Jul 11, 2023
@parth-gr parth-gr requested a review from BlaineEXE July 12, 2023 14:45
Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

@BlaineEXE Do you recall why we implemented the copy-binaries command instead of using cp like this?

@BlaineEXE
Copy link
Member

I don't recall why I implemented it as a daemon. It used to copy tini as well as rook, but that's no longer needed. I think I may have implemented it as a binary simply because the behavior used to be accomplished by the OSD daemon process and not considered cp as a good method. TBH, I think this is a much better approach.

@BlaineEXE
Copy link
Member

This is not urgent, so I will wait for @travisn to get back from vacation. I think it may make sense to merge this into master and not backport it so that we have some time to make sure it does not negatively affect anything in master. I don't anticipate any issues, but there is always a small chance.

remove copy-binaries cmd and related code

Signed-off-by: Sheetal Pamecha <spamecha@redhat.com>
@Sheetalpamecha
Copy link
Contributor Author

No code change done, forced pushed to rerun all failing test case.

@travisn travisn merged commit d5f6184 into rook:master Jul 31, 2023
42 of 49 checks passed
travisn added a commit that referenced this pull request Jul 31, 2023
osd: use cp -a command for copying init-containers binary (backport #12501)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

init-copy-binaries can leave container in a bad state
4 participants