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

Update che.sh #1329

Merged
merged 4 commits into from
May 23, 2016
Merged

Update che.sh #1329

merged 4 commits into from
May 23, 2016

Conversation

TylerJewell
Copy link

@TylerJewell TylerJewell commented May 22, 2016

_1 Upvote_ CHE-1202 - Add in checks in case 'docker ps' fails. There are two additional checks. First, verify that group and owner have rw permissions for /var/run/docker.sock. Second, verify that the docker client version and the docker server version match. In situations where docker client is in a Che container and that container is using a host's docker daemon, it is possible for the client and server to not match.

@garagatyi, @eivantsov what do you think?

I don't have a way to generate good tests for this. I did the best that I could in a VM, but all the VMs I create generally don't have either of these issues. Do we have good environments to test for this improvement?

Tyler Jewell added 2 commits May 22, 2016 14:01
Add in catch all error message.

# CHE-1202: Improve error messages in case of docker ps failure
# Verify that /var/run/docker.sock has owner and group read / write permissions
PERMS=$(stat -c %A /var/run/docker.sock)

Choose a reason for hiding this comment

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

  1. Even if docker.sock has owner/group rw permission it doesn't mean that current user is owner or included in needed group.
  2. docker.sock can have rw permission for others.
  3. sudo chmod 660 /var/run/docker.sock will help only when user has needed id or gid

Copy link
Author

Choose a reason for hiding this comment

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

We perform the uid check elsewhere in the script. It is checked before getting to this step.

@codenvy-ci
Copy link

@TylerJewell
Copy link
Author

@garagatyi Added formatting.

@codenvy-ci
Copy link

Build # 664 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/664/ to view the results.

@garagatyi
Copy link

LGTM

@garagatyi
Copy link

Please use squash when you will be ready to merge

@TylerJewell
Copy link
Author

@eivantsov Added check for other group.

@ghost
Copy link

ghost commented May 23, 2016

+1

@TylerJewell TylerJewell merged commit 1ae0920 into master May 23, 2016
@TylerJewell TylerJewell deleted the CHE-1202 branch May 23, 2016 16:24
@codenvy-ci
Copy link

Build # 678 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/678/ to view the results.

AndrienkoAleksandr added a commit to eclipse-che/che-theia that referenced this pull request May 7, 2019
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
AndrienkoAleksandr added a commit to eclipse-che/che-theia that referenced this pull request May 13, 2019
* Use cwd for terminal creation

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* Use cwd only for editor container. Limited by eclipse-che/che#1329

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* Clean up.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
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.

None yet

3 participants