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

Document how to fix file permission errors while using odo with GKE/AKS/EKS #6840

Conversation

valaparthvi
Copy link
Contributor

@valaparthvi valaparthvi commented May 24, 2023

What type of PR is this:
/area documentation

What does this PR do / why we need it:

Which issue(s) this PR fixes:

Fixes #6821

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:
https://deploy-preview-6840--odo-docusaurus-preview.netlify.app/docs/user-guides/advanced/using-odo-with-other-clusters

@netlify
Copy link

netlify bot commented May 24, 2023

Deploy Preview for odo-docusaurus-preview ready!

Name Link
🔨 Latest commit d5eb9ba
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/6474723a36ad4f00082595d1
😎 Deploy Preview https://deploy-preview-6840--odo-docusaurus-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@openshift-ci openshift-ci bot added the area/documentation Issues or PRs related to documentation or the 'odo.dev' website label May 24, 2023
@openshift-ci openshift-ci bot requested review from feloy and kadel May 24, 2023 13:16
@valaparthvi valaparthvi force-pushed the 6821-document-how-to-use-sourcemapping-to-fix-permission-errors branch 2 times, most recently from 30a41e7 to 5403913 Compare May 24, 2023 13:21
@odo-robot
Copy link

odo-robot bot commented May 24, 2023

OpenShift Unauthenticated Tests on commit ccc2800 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented May 24, 2023

NoCluster Tests on commit ccc2800 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented May 24, 2023

Unit Tests on commit ccc2800 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented May 24, 2023

Validate Tests on commit ccc2800 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented May 24, 2023

Kubernetes Tests on commit ccc2800 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented May 24, 2023

Windows Tests (OCP) on commit ccc2800 finished with errors.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented May 24, 2023

OpenShift Tests on commit ccc2800 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented May 24, 2023

Kubernetes Docs Tests on commit 85b4ff9 finished successfully.
View logs: TXT HTML

Signed-off-by: Parthvi Vala <pvala@redhat.com>
@valaparthvi valaparthvi force-pushed the 6821-document-how-to-use-sourcemapping-to-fix-permission-errors branch from 5403913 to 3ede01e Compare May 25, 2023 11:39
Comment on lines 142 to 146
All the Devfiles provided by the [Devfile Registry](https://registry.devfile.io) use container image set a non-root user, which is why we see these permission related issues; we usually do not see this for a root user.

When a user does not have access to all the locations in a system, we need to provide a way to use a location where the user can read/write. There are 2 parts to this solution as well:
1. Set `sourceMapping` of the Devfile `container` component to a location where the user has read/write access.
2. If a Devfile `volume` component is defined, then ensure that the `.volumeMounts.path` of the Devfile container component where the volume will be mount is in a location where the user has read/write access.
Copy link
Contributor

Choose a reason for hiding this comment

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

With this solution, the files will be written in the container's filesystem, which is not optimal:

For example, when setting sourceMapping to java-maven-app as in the example below, this directory will be created in a directory outside of /projects (in the current directory specified in the container's image).

If you set sourceMapping to /projects/java-maven-app, you get the same permission problems.

This solution inhibits the use of a volume (either persistent or volatile) for the directory where the sources are placed, as we won't use this volume.

Comment on lines 319 to 320
But this solution may not always be feasible, especially while dealing with large filesystems.
>Be cautious with the use of fsGroup. The changing of group ownership of an entire volume can cause pod startup delays for slow and/or large filesystems. It can also be detrimental to other processes that share the same volume if their processes do not have access permissions to the new GID. For this reason, some providers for shared file systems such as NFS do not implement this functionality. These settings also do not affect ephemeral volumes.
Copy link
Contributor

Choose a reason for hiding this comment

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

The second part of the warning (sharing the volume with other processes) is not related to how odo uses the volume, as odo is creating a dedicated volume for the component, which won't be shared with others.

For the first part of the warning, we can explain that this will have an effect only at the "restart" of the pod (when the user modifies the container's information in the Devfile). In a normal use (without modifying the container in Devfile), there won't be any effect, as the Volume is empty when it is mounted the first time, and only populated with the sync.

These warnings are valid for containers that mount volumes with persistent data (imagine a Postgres image mounting a 1Tb volume with its data), but I think we can mitigate them in the case of odo.

valaparthvi and others added 3 commits May 26, 2023 20:34
Co-authored-by: Armel Soro <armel@rm3l.org>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
@sonarcloud
Copy link

sonarcloud bot commented May 29, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

No Coverage information No Coverage information
0.3% 0.3% Duplication

@feloy
Copy link
Contributor

feloy commented May 30, 2023

LGTM. Thanks @valaparthvi for this work

@rm3l feel free to approve it when you are ok with the changes

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label May 30, 2023
@rm3l
Copy link
Member

rm3l commented May 30, 2023

Doc-only PR.

/override windows-integration-test/Windows-test

@openshift-ci
Copy link

openshift-ci bot commented May 30, 2023

@rm3l: Overrode contexts on behalf of rm3l: windows-integration-test/Windows-test

In response to this:

Doc-only PR.

/override windows-integration-test/Windows-test

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Issues or PRs related to documentation or the 'odo.dev' website lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document troubleshooting storage permission issues with GKE/AKS/EKS
4 participants