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

restructure ci #34

Open
wants to merge 75 commits into
base: develop
Choose a base branch
from
Open

restructure ci #34

wants to merge 75 commits into from

Conversation

yajith
Copy link
Contributor

@yajith yajith commented Sep 25, 2024

new ci workflow and Dockerfile updates to achieve multi-stage builds, avoiding the need for maintaining a shell script for build the containers.

Copy link
Member

@deviantony deviantony left a comment

Choose a reason for hiding this comment

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

Can likely also remove the build_and_push.sh script as part of this PR.

ARG GIT_COMMIT=unspecified
FROM portainer/base:latest
ARG GO_VERSION=1.22.7
Copy link
Member

Choose a reason for hiding this comment

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

Whilst this is the latest patch version for 1.22, I believe this is not aligned with other software which is built upon 1.22.6.

I'd standardize over a single version and make sure that you have a tracking sheet / doc somewhere that tells you where this version is defined so that you can easily update it in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to fix some of the recent "High" CVE 1.22.7 bump was done for 2.22.x(=develop) about 2 weeks ago.
https://linear.app/portainer/issue/BE-11188/2024-september-cve (34158 etc.)

I added that ARG GO_VERSION=1.22.7 with that in mind, just as a default. I will drop it to 1.22.6 as the "default"
Overall, my intention was to override that as a build arg and bump the version from outside the Dockerfile.

Copy link
Member

Choose a reason for hiding this comment

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

All good I missed that during my holidays. I'll let you manage the Go version but make sure it is consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RUN go mod download -x

# Build the Go application
RUN GOOS=${TARGETOS} GOARCH=${TARGETARCH} CGO_ENABLED=0 go build --installsuffix cgo --ldflags '-s' -o portainer-updater
Copy link
Member

Choose a reason for hiding this comment

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

Why not just use the Makefile and make in the CI ? Are you experimenting to gear toward multi-stage builds across the software ? I don't recall this being something we're doing for other software.

Just a question, I'm not particularly against it, just curious. I always prefer consistency.

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, it was exactly that! I was hoping to work on making multi-stage builds for everything, I know Steven also had this in mind for quite some time now. I used portainer-updater as a starting point because it is probably less complex compared to portainer or agent, wanted to start from a smaller unit and work up.

Also having everything consolidated in a multi-stage Dockerfile lets us use official docker github actions for "build-and-push" and others...which is probably a better and standardized approach. Quite keen to know any comments/suggestions etc.

I didn't remove the build-and-push script and other supporting secrets etc. from GitHub until further discussion is had about this direction.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me, as long as you've checked that we are producing a similar image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

still a work in progress, there are certain things about the image manifest I haven't been able to solve, which need needs figuring out.

@yajith yajith force-pushed the chore/restructure-ci branch from a1e887e to 2d02f82 Compare December 21, 2024 04:37
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.

2 participants