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

Change flow-aggregator base images to reduce image size #2004

Merged

Conversation

hanlins
Copy link
Contributor

@hanlins hanlins commented Mar 27, 2021

Flow aggregator only needs the Golang binary and doesn't have dependencies on OVS. Changing the base image to reduce image size.

Closes #1986.

@hanlins
Copy link
Contributor Author

hanlins commented Mar 27, 2021

Found hack/netpol/Dockerfile was using alpine, maybe should switch to scratch?

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

We cannot use alpine as a base image for VMware originated open-source software. We should fix the netpol image as well but at least it is only used for testing.

I suggested using distroless in the issue. Please use scratch since we are only including a static Go binary.

@hanlins hanlins force-pushed the fix/1986/flow-aggregator-base-img branch from c9023a8 to 732262f Compare March 27, 2021 03:02
@hanlins
Copy link
Contributor Author

hanlins commented Mar 27, 2021

kk, fixed.

@codecov-io
Copy link

codecov-io commented Mar 27, 2021

Codecov Report

Merging #2004 (4333de5) into main (aaaea4f) will increase coverage by 0.20%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2004      +/-   ##
==========================================
+ Coverage   65.14%   65.34%   +0.20%     
==========================================
  Files         197      197              
  Lines       17407    17407              
==========================================
+ Hits        11340    11375      +35     
+ Misses       4880     4858      -22     
+ Partials     1187     1174      -13     
Flag Coverage Δ
kind-e2e-tests 56.51% <ø> (+0.31%) ⬆️
unit-tests 41.69% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/ovs/ovsconfig/ovs_client.go 50.10% <0.00%> (+0.61%) ⬆️
...ntroller/networkpolicy/networkpolicy_controller.go 71.75% <0.00%> (+0.64%) ⬆️
pkg/agent/nodeportlocal/k8s/npl_controller.go 62.34% <0.00%> (+1.21%) ⬆️
pkg/agent/cniserver/server.go 70.51% <0.00%> (+1.92%) ⬆️
...gent/controller/networkpolicy/status_controller.go 75.34% <0.00%> (+2.73%) ⬆️
pkg/agent/cniserver/pod_configuration.go 57.42% <0.00%> (+3.51%) ⬆️
pkg/agent/cniserver/ipam/ipam_delegator.go 53.48% <0.00%> (+4.65%) ⬆️
pkg/agent/cniserver/ipam/ipam_service.go 68.42% <0.00%> (+5.26%) ⬆️
pkg/monitor/controller.go 57.57% <0.00%> (+6.06%) ⬆️
pkg/apiserver/handlers/endpoint/handler.go 70.58% <0.00%> (+11.76%) ⬆️

antoninbas
antoninbas previously approved these changes Mar 27, 2021
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR. If you feel like opening a second one, feel free to fix the netpol base image :)

Copy link
Member

@srikartati srikartati left a comment

Choose a reason for hiding this comment

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

LGTM.
I see that netpol image is also being changed. It would be good to change PR title and description.

@@ -7,7 +7,7 @@ COPY . /antrea

RUN make flow-aggregator

FROM antrea/base-ubuntu:${OVS_VERSION}
FROM scratch
Copy link
Member

Choose a reason for hiding this comment

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

/cc @dreamtalen @zyiou who contributes to Flow Aggregator feature.

@@ -4,6 +4,6 @@ ADD ./ /netpol
WORKDIR /netpol
RUN go build -o main pkg/main/main.go

FROM alpine
FROM scratch
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggested doing this in a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, will revert that change, thx!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, will put the change for netpol image in another PR.

@hanlins hanlins changed the title Change flow-aggregator base image to reduce image size Change base images to reduce image size Mar 29, 2021
@hanlins hanlins force-pushed the fix/1986/flow-aggregator-base-img branch from 4333de5 to 74b8b6c Compare March 29, 2021 18:33
@hanlins hanlins changed the title Change base images to reduce image size Change flow-aggregator base images to reduce image size Mar 29, 2021
@antoninbas
Copy link
Contributor

/test-e2e

@antoninbas
Copy link
Contributor

jenkins-e2e is passing
/skip-all

@antoninbas antoninbas merged commit 08ea67c into antrea-io:main Mar 29, 2021
dreamtalen added a commit to dreamtalen/antrea that referenced this pull request Mar 31, 2021
Fix the error of Flow Aggregator image after PR antrea-io#2004 changed the base
image to scratch.
Change the image name target "flow-aggregator-ubuntu" to
"flow-aggregtor-image" in Makefile since ubuntu is not relevant.
Change the base image of Flow Aggregator code coverage image to
ubuntu:20.04.
dreamtalen added a commit to dreamtalen/antrea that referenced this pull request Mar 31, 2021
Fix the error of Flow Aggregator image after PR antrea-io#2004 changed the base
image to scratch.
Change the image name target "flow-aggregator-ubuntu" to
"flow-aggregtor-image" in Makefile since ubuntu is not relevant.
Change the base image of Flow Aggregator code coverage image to
ubuntu:20.04.
dreamtalen added a commit to dreamtalen/antrea that referenced this pull request Mar 31, 2021
Fix the error of Flow Aggregator image after PR antrea-io#2004 changed the base
image to scratch.
Change the image name target "flow-aggregator-ubuntu" to
"flow-aggregtor-image" in Makefile since ubuntu is not relevant.
Change the base image of Flow Aggregator code coverage image to
ubuntu:20.04.
dreamtalen added a commit to dreamtalen/antrea that referenced this pull request Apr 1, 2021
Fix the error of Flow Aggregator image after PR antrea-io#2004 changed the base
image to scratch.
Change the image name target "flow-aggregator-ubuntu" to
"flow-aggregtor-image" in Makefile since ubuntu is not relevant.
Change the base image of Flow Aggregator code coverage image to
ubuntu:20.04.
dreamtalen added a commit to dreamtalen/antrea that referenced this pull request Apr 1, 2021
Fix the error of Flow Aggregator image after PR antrea-io#2004 changed the base
image to scratch.
Change the image name target "flow-aggregator-ubuntu" to
"flow-aggregtor-image" in Makefile since ubuntu is not relevant.
Change the base image of Flow Aggregator code coverage image to
ubuntu:20.04.
srikartati pushed a commit that referenced this pull request Apr 1, 2021
…2016)

Fix the error of Flow Aggregator image after PR #2004 changed the base
image to scratch.
Change the image name target "flow-aggregator-ubuntu" to
"flow-aggregtor-image" in Makefile since ubuntu is not relevant.
Change the base image of Flow Aggregator code coverage image to
ubuntu:20.04.
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.

Does the flow-aggregator image need to depend on antrea/base-ubuntu?
5 participants