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

make builder image version consistent with glide.lock hash #1621

Merged
merged 1 commit into from
Sep 24, 2018

Conversation

kunmingg
Copy link
Contributor

@kunmingg kunmingg commented Sep 24, 2018

To simplify test and local dev.

Old method:
go pkg was compiled in builder image:

  1. every line of change requires build two images: builder and final image.
  2. resulting in need of edit builder image version in Makefile every time before run make build
  3. resulting in frequent version change of builder image.

New method:
builder image is cache of glide & vendor dir, glide.lock file 100% determines the content of builder image. So use hash code of glide.lock as builder image version. And resolve 1,2,3 of old method


This change is Reviewable

COPY ./ $GOPATH/src/github.com/kubeflow/kubeflow/bootstrap/

RUN go build ${GOLANG_GCFLAGS} -i -o /opt/kubeflow/bootstrapper \
${GOPATH}/src/github.com/kubeflow/kubeflow/bootstrap/cmd/bootstrap/main.go

FROM golang:${GOLANG_VERSION} AS build
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 this line for?
Do we need to build after this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which line?

@@ -18,10 +18,3 @@ RUN mkdir -p $GOPATH/src/github.com/kubeflow/kubeflow/bootstrap
COPY ./glide* $GOPATH/src/github.com/kubeflow/kubeflow/bootstrap/
RUN cd $GOPATH/src/github.com/kubeflow/kubeflow/bootstrap/ && \
glide install -v

Copy link
Contributor

Choose a reason for hiding this comment

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

So builder only containers vendor, and we only need to rebuild it if glide is changed, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

@lluunn
Copy link
Contributor

lluunn commented Sep 24, 2018

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lluunn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 320c379 into kubeflow:master Sep 24, 2018
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
surajkota pushed a commit to surajkota/kubeflow that referenced this pull request Jun 13, 2022
* add kfp-tekton deployment and update IBM stacks

* update owner file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants