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

Make user-data-permission-fix optional #3994

Merged

Conversation

nerdinand
Copy link
Contributor

@nerdinand nerdinand commented Dec 7, 2021

Motivation and context

Deploying CVAT to OpenShift (see also #3992), we've run into problems with the user-data-permission-fix initContainer. OpenShift seems to be stricter in what you can do permission-wise. To mitigate this, we propose making the permission fix optional. Along with adding RUN /bin/chmod -R 777 ${HOME} to the Dockerfile, this works for us.

How has this been tested?

Helm chart tested manually

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

@nerdinand
Copy link
Contributor Author

Maybe a better solution would be to add RUN /bin/chmod -R 777 ${HOME} to the Dockerfile in the first place, then this initContainer might become obsolete altogether. Thoughts?

@nmanovic
Copy link
Contributor

@azhavoro , @ActiveChooN , could you please look?

@ActiveChooN
Copy link
Contributor

Maybe a better solution would be to add RUN /bin/chmod -R 777 ${HOME} to the Dockerfile in the first place, then this initContainer might become obsolete altogether. Thoughts?

If this fix can be applied in the build stage it looks like a better solution.

@nmanovic
Copy link
Contributor

@azhavoro , could you please review and share your opinion? Should we merge the PR?

@nmanovic nmanovic removed the request for review from ActiveChooN January 11, 2022 13:21
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@azhavoro azhavoro left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nmanovic nmanovic left a comment

Choose a reason for hiding this comment

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

@nerdinand , thanks for the contribution.

@nmanovic nmanovic merged commit eb7e719 into cvat-ai:develop Jan 12, 2022
@nmanovic nmanovic mentioned this pull request Mar 4, 2022
7 tasks
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