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

Switching autoscaler-webhook to utilize distroless as base Image #3276

Merged
merged 7 commits into from
Jul 22, 2023

Conversation

Kalaiselvi84
Copy link
Contributor

What type of PR is this?

Uncomment only one /kind <> line, press enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug

/kind cleanup

/kind documentation
/kind feature
/kind hotfix
/kind release

What this PR does / Why we need it:

Which issue(s) this PR fixes:

Closes #909

Special notes for your reviewer:

@github-actions github-actions bot added the kind/cleanup Refactoring code, fixing up documentation, etc label Jul 19, 2023
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: d32fd147-4bce-4f9b-a203-338fe643b306

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

Good start!

Can you also have a look through our docs to see what references the autoscaler-webhook container image, and update that as well?

We'll also need to manually test to make sure it still works please.

@@ -22,7 +22,7 @@ RUN go mod init && go mod tidy
RUN CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo -o server .

# Create the final image that will run the webhook server for FleetAutoscaler webhook policy
FROM alpine:3.14
FROM gcr.io/distroless/static-debian11:nonroot
RUN adduser -D -u 1000 server
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
RUN adduser -D -u 1000 server
WORKDIR /

No need for this anymore, and we move everything into root.

@@ -22,7 +22,7 @@ RUN go mod init && go mod tidy
RUN CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo -o server .

# Create the final image that will run the webhook server for FleetAutoscaler webhook policy
FROM alpine:3.14
FROM gcr.io/distroless/static-debian11:nonroot
RUN adduser -D -u 1000 server

COPY --from=builder /go/src/autoscaler-webhook \
Copy link
Member

Choose a reason for hiding this comment

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

change this to copy to /server

@@ -31,5 +31,5 @@ COPY --from=builder /go/src/autoscaler-webhook \
RUN chown -R server /home/server && \
Copy link
Member

Choose a reason for hiding this comment

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

This can all go away.

USER 1000
ENTRYPOINT /home/server/server
USER nonroot:nonroot
ENTRYPOINT ["/home/server/server"]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ENTRYPOINT ["/home/server/server"]
ENTRYPOINT ["/server"]

@google-oss-prow google-oss-prow bot added size/S and removed size/XS labels Jul 19, 2023
@Kalaiselvi84
Copy link
Contributor Author

Kalaiselvi84 commented Jul 19, 2023

Can you also have a look through our docs to see what references the autoscaler-webhook container image, and update that as well?

Do we have a need to update all files , excluding release blogs, that currently using "autoscaler-webhook 0.6" to instead use "autoscaler-webhook 0.7"?

@markmandel
Copy link
Member

Do we have a need to update all files , excluding release blogs, that currently using "autoscaler-webhook 0.6" to instead use "autoscaler-webhook 0.7"?

Yes please!

@Kalaiselvi84
Copy link
Contributor Author

We'll also need to manually test to make sure it still works please.

could you please provide more context?

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 7ed1de35-e84f-4e51-96ef-85a1b8ef131d

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@markmandel
Copy link
Member

could you please provide more context?

Ah yes - excellent question!

We use this image in this quickstart: https://agones.dev/site/docs/getting-started/create-webhook-fleetautoscaler/

So if you can go through this quickstart, but utilise your new image rather than the old one, that would be perfect.

Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

Chatted with @Kalaiselvi84 offline. Tested, and looks good 👍🏻

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Kalaiselvi84, markmandel

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

@markmandel markmandel enabled auto-merge (squash) July 21, 2023 18:53
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: ebe73d41-4e88-494d-a83f-fd5b300573a4

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@markmandel
Copy link
Member

--- FAIL: TestAllocatorAllocateOnGameServerUpdateError (19.31s)
    allocator_test.go:499: 
        	Error Trace:	/go/src/agones.dev/agones/pkg/gameserverallocations/allocator_test.go:499
        	Error:      	An error is expected but got nil.
        	Test:       	TestAllocatorAllocateOnGameServerUpdateError

flake. I thought I fixed this.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 3396222b-a8c2-4c9e-81a0-f16d73ac52b8

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@markmandel
Copy link
Member

@Kalaiselvi84 is the image pushed yet? If not, that would explain why the e2e tests are failing 😄

@Kalaiselvi84
Copy link
Contributor Author

@Kalaiselvi84 is the image pushed yet? If not, that would explain why the e2e tests are failing 😄

I used go mod init to create the go.mod and go.sum files, but unfortunately, the issue persists when attempting to run the make push command.

@google-oss-prow google-oss-prow bot removed the lgtm label Jul 21, 2023
@google-oss-prow
Copy link

New changes are detected. LGTM label has been removed.

@google-oss-prow google-oss-prow bot added size/L and removed size/S labels Jul 21, 2023
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: e6b7406c-c6f6-4419-8f85-248dd2681349

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 957feef5-77f9-4582-b789-3667f8b06245

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/3276/head:pr_3276 && git checkout pr_3276
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.34.0-dev-59bffb6-amd64

@markmandel markmandel merged commit 8f575ed into googleforgames:main Jul 22, 2023
2 checks passed
@Kalaiselvi84 Kalaiselvi84 deleted the issues/291828200 branch March 15, 2024 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved kind/cleanup Refactoring code, fixing up documentation, etc size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should we rebase images from alpine to gcr.io/distroless/static ?
3 participants