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

Fix closing exec connections on che task SIGINT termination. #51

Conversation

AndrienkoAleksandr
Copy link
Contributor

Referenced issue:

eclipse-che/che#14887

Fix closing exec connections on che task SIGINT termination.

Proposed changes: change image from scratch to busybox. Image will be bigger on 1.2Mb (26.7Mb against 25.5), but issue won't reproduces with this image.
P.S.: Alpine 3.10.2 works fine too, but image is bigger.

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

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@benoitf
Copy link
Contributor

benoitf commented Nov 4, 2019

it might be an issue for CRW as before it was scratch while now it's not rhel based

@@ -12,17 +12,16 @@
# Dockerfile defines che-machine-exec production image eclipse/che-machine-exec-dev
#

FROM golang:1.10.3-alpine as builder
FROM golang:1.10.7-alpine as builder
WORKDIR /go/src/github.com/eclipse/che-machine-exec/
COPY . .
RUN CGO_ENABLED=0 GOOS=linux go build -a -ldflags '-w -s' -a -installsuffix cgo -o che-machine-exec .
RUN apk add --no-cache ca-certificates

RUN adduser -D -g '' unprivilegeduser
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the meaning of this instruction as we're in another image/container at runtime ?

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@benoitf
Copy link
Contributor

benoitf commented Nov 4, 2019

@AndrienkoAleksandr maybe there was a misunderstanding while I clicked on the line to report the question

it was about:

RUN adduser -D -g '' unprivilegeduser

instruction, not the update of golang

my question was, why do we add a user in the builder image as we don't use it on runtime ?

@AndrienkoAleksandr
Copy link
Contributor Author

@benoitf why do we add a user in the builder image as we don't use it on runtime ?

Hello, we have a copy /etc/passwd to the builder image and define this user before start ENTRYPOINT. I take a look more and I see this stuff works only for docker. With openshift we used another user without username and this user in the root group. I think we can remove unprivilegeduser from dockerfile, because we really don't use it. What do you think?

@benoitf
Copy link
Contributor

benoitf commented Nov 4, 2019

@AndrienkoAleksandr
yes now it is used because you added back the copy of /etc/passwd line
but when I commented, you removed the copy of etc/passwd file

@AndrienkoAleksandr
Copy link
Contributor Author

but when I commented, you removed the copy of etc/passwd file

Sorry, yes, removing it was bad change.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@AndrienkoAleksandr
Copy link
Contributor Author

Close on favor #54

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