Skip to content

CHE-10756: Use regular user instead of root in theia image#11076

Merged
mmorhun merged 2 commits intomasterfrom
CHE-10756
Sep 6, 2018
Merged

CHE-10756: Use regular user instead of root in theia image#11076
mmorhun merged 2 commits intomasterfrom
CHE-10756

Conversation

@mmorhun
Copy link
Copy Markdown
Contributor

@mmorhun mmorhun commented Sep 5, 2018

Signed-off-by: Mykola Morhun mmorhun@redhat.com

What does this PR do?

Before we used root in Theia image. It is not good way to go and moreover caused some problems, for example, with yo generator.
This PR adds regular user theia and runs Theia IDE on behalf of the user.

What issues does this PR fix or reference?

#10756

Tested on docker infrastructure and openshift.io

@mmorhun mmorhun self-assigned this Sep 5, 2018
@mmorhun mmorhun requested review from a user, AndrienkoAleksandr and benoitf September 5, 2018 12:25
@mmorhun mmorhun requested a review from riuvshin as a code owner September 5, 2018 12:25
@benoitf benoitf added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Sep 5, 2018
set -e
set -u

yum install -y sudo
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if possible, merge it with other yum install instruction post github-check

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To me it is clearer when we install components in its logical group/place.
Just curious, what is advantage of such merging?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

speed of build (like searching for mirrors again)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Aren't they cached?

@benoitf
Copy link
Copy Markdown
Contributor

benoitf commented Sep 5, 2018

ci-build

@benoitf
Copy link
Copy Markdown
Contributor

benoitf commented Sep 5, 2018

@mmorhun could you rebase changes to check the e2e tests are passing ?

Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
@mmorhun
Copy link
Copy Markdown
Contributor Author

mmorhun commented Sep 5, 2018

@benoitf done

tests

Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
@benoitf
Copy link
Copy Markdown
Contributor

benoitf commented Sep 5, 2018

ci-build

Copy link
Copy Markdown
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

Please squash the commits

@mmorhun mmorhun merged commit bb44854 into master Sep 6, 2018
@mmorhun mmorhun deleted the CHE-10756 branch September 6, 2018 06:32
@benoitf benoitf added this to the 6.11.0 milestone Sep 6, 2018
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Outline of a bug - must adhere to the bug report template.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants