Skip to content

Conversation

@chatton
Copy link
Contributor

@chatton chatton commented Apr 21, 2021

All Submissions:

This PR refactors the agent Dockerfiles to now use a context builder.

  • Have you signed our CLA?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

@chatton chatton changed the base branch from master to CLOUDP-83092_add_release_task_for_init_containers April 21, 2021 15:52
Base automatically changed from CLOUDP-83092_add_release_task_for_init_containers to master April 22, 2021 09:21
@chatton chatton marked this pull request as ready for review April 22, 2021 11:05
@chatton chatton requested review from bznein and rodrigovalin April 22, 2021 11:05
Copy link
Contributor

@rodrigovalin rodrigovalin left a comment

Choose a reason for hiding this comment

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

LGTM with one comment.

  • no blocker: I think the agent's builder Docker file can be made a little less verbose by using ADD, it can be done easily

- tools_version
- tools_distro
- agent_distro
- noop
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you fixed this already :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we haven't yet released 0.10.0, but let's do that now, no harm!

@@ -0,0 +1,18 @@
FROM curlimages/curl:7.76.1 as builder
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this one can be replaced by a scratch image like

FROM scratch

ADD https://mciuploads.s3.amazonaws.com/mms-automation/mongodb-mms-build-agent/builds/automation-agent/prod/mongodb-mms-automation-agent-${agent_version}.${agent_distro}.tar.gz /data/mongodb-agent.tgz
ADD https://downloads.mongodb.org/tools/db/mongodb-database-tools-${tools_distro}-${tools_version}.tgz /data/mongodb-tools.tgz

ADD agent/LICENSE /data/licenses

Instead of the multi-stage build with curl.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah much better!

Comment on lines 18 to 19
COPY --from=base /data/mongodb-agent.tar.gz agent
COPY --from=base data/mongodb-tools.tgz .
Copy link
Contributor

Choose a reason for hiding this comment

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

Sources and destinations are really different here. Why is that? It is kind of confusing.

&& chmod ugo+rw /var/log/mongodb-mms-automation/readiness.log


COPY --from=base /data/mongodb-agent.tar.gz agent
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use absolute directories for the destination? Or it will depend on current WORKDIR

@chatton
Copy link
Contributor Author

chatton commented Apr 23, 2021

@rodrigovalin updated based on your feedback! Everything is using absolute paths now.

@rodrigovalin
Copy link
Contributor

LGTM

@chatton chatton merged commit 837aaec into master Apr 23, 2021
@chatton chatton deleted the CLOUDP-83092_add_context_image_for_agents branch April 23, 2021 16:39
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.

3 participants