feat(ws): Properly containerize backend component #323 #386
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Liav Weiss (EXT-Nokia) <[email protected]>
liavweiss
force-pushed
the
backend_dockerfile/#323
branch
from
0ffaa72 to
a89a08e
Compare
| RUN go mod edit -replace=github.com/kubeflow/notebooks/workspaces/controller=./controller | ||
|
|
||
| # Download dependencies | ||
| RUN go mod download |
There was a problem hiding this comment.
These back-to-back RUN commands can be more efficiently combined to reduce a spurious layer
RUN go mod edit -replace=github.com/kubeflow/notebooks/workspaces/controller=./controller && \
go mod download
| COPY backend/cmd/ cmd/ | ||
| COPY backend/api/ api/ | ||
| COPY backend/internal/ internal/ | ||
| COPY backend/openapi/ openapi/ | ||
|
|
||
| # Build the Go application | ||
| RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -a -o backend ./cmd/main.go |
There was a problem hiding this comment.
Unrelated to changes in this PR - but I'm confused by the use case of specifying TARGETARCH here...
Our Makefile never sets this build argument... so do we even need it? Under what circumstance would it need to be defined?
| @@ -110,7 +111,7 @@ docker-buildx: ## Build and push docker image for the manager for cross-platform | |||
| sed -e '1 s/\(^FROM\)/FROM --platform=\$$\{BUILDPLATFORM\}/; t' -e ' 1,// s//FROM --platform=\$$\{BUILDPLATFORM\}/' Dockerfile > Dockerfile.cross | |||
There was a problem hiding this comment.
Again, unrelated to the changes in this PR - but I am confused as to why we have 2 separate expressions ( -e ) in this sed command...
Our Dockerfile has a comment on Line 1 - so (at least today) - the initial -e command will never pass. And since we are not relying on that initial command - unclear why we even attempt to support it (?)
Clearly the 2nd expression is "robust enough" - as we are relying on it today...
|
In order to properly test this change - I wanted to get the container image running in k8s The following was my testing methodology:
All API calls tested above returned successfully with expected JSON payloads reflecting the state of my cluster! |
closes: #323
In this pr I fixed the make docker-build + make docker-buildx.
NOTE: When you run "make docker-buildx" you will get this ERROR:
"ERROR: failed to solve: failed to push nbv2-backend:latest: push access denied, repository does not exist or may require authorization: server message: insufficient_scope: authorization failed
make: [Makefile:135: docker-buildx] Error 1 (ignored)",
Because we didn't specify the right registry in the Makefile.