-
Notifications
You must be signed in to change notification settings - Fork 104
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
generate the polycubectl binary for pcn_k8s docker image #199
Conversation
@goldenrye Could you please add some description to this PR?! Thanks! |
Didn't realize there is no description, just add it and thanks for the reminding. |
@@ -3,7 +3,7 @@ FROM ubuntu:18.04 | |||
ARG MODE=default | |||
RUN --mount=target=/polycube cp -r /polycube /tmp/polycube && \ | |||
cd /tmp/polycube && \ | |||
SUDO="" WORKDIR="/tmp/dev" ./scripts/install.sh $MODE && \ | |||
SUDO="" USER="root" WORKDIR="/tmp/dev" ./scripts/install.sh $MODE && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to specify the user as "root"? If I'm not mistaken as long as the user have sudo privilege, it should be fine.
@mauriciovasquezbernal any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the problem here is that without setting the USER environment variable, docker building will fail in this line silently,
install(CODE
"execute_process( COMMAND /bin/sh -c "mkdir -p /var/log/polycube && chown $ENV{USER} /var/log/polycube ")")
@goldenrye Why do we want this inside the k8s-pod-network, isn't the Try this and let us know if this what you want.
|
This command works but need add option "--net=host", but you have to run it in each node and won't be able to do that via kubectl like, polycube-sddh5 is a pod running in a slave node |
This relates to bug report #198. |
Hi, in order to use
It always worked fine. |
To me this PR makes sense. |
I didn't enable this at that time because we're worried about the size of the image. I don't think this is an issue anymore, also the |
@goldenrye It looks there's a consensus to add this PR. I would suggest to add a brief text in the "troubleshooting" section here, so that people are aware of this possibility: |
8ea65b0
to
c9617c1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
By default there is no polycubectl binary included in our official published pcn-k8s docker image: https://hub.docker.com/r/polycubenetwork/k8s-pod-network. We need this binary to help user to get more information of cubes running in the docker.