Skip to content

Update Dockerfile#36

Merged
matburt merged 1 commit intoproject-receptor:masterfrom
elyezer:update-dockerfile
Nov 1, 2019
Merged

Update Dockerfile#36
matburt merged 1 commit intoproject-receptor:masterfrom
elyezer:update-dockerfile

Conversation

@elyezer
Copy link
Copy Markdown
Member

@elyezer elyezer commented Oct 25, 2019

Do the following:

  • Use Fedora 30 as the base image
  • Install receptor from source rather than wheel
  • Set /var/lib/receptor as the home directory for receptor

Copy link
Copy Markdown
Member

@matburt matburt left a comment

Choose a reason for hiding this comment

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

I think we're going to need to find a better way to propogate the code into the image than adding the dev directory.

Comment thread packaging/docker/Dockerfile Outdated
@elyezer
Copy link
Copy Markdown
Member Author

elyezer commented Oct 28, 2019

@matburt I've updated the PR to remove the /receptor from the container file system. I saw about multi-stage build (https://docs.docker.com/develop/develop-images/multistage-build/) and that may allow us to build the receptor wheel and get it installed. I will continue taking a look how we could improve the container image and will send a PR later if I found something better to use.

@elyezer
Copy link
Copy Markdown
Member Author

elyezer commented Oct 28, 2019

I found that running dnf update is adding about 80MB to the image but I think that is advised to ensure everything is up to date and with all the security fixes in. Any concerns/suggestions about that?

@matburt
Copy link
Copy Markdown
Member

matburt commented Oct 29, 2019

The problem is you're adding the entire dev directory to the image, which would include the .git directory. The goal of this dockerfile is to generate a functional production image of Receptor so we need to generate a whl or something and install it into this.

@matburt
Copy link
Copy Markdown
Member

matburt commented Oct 29, 2019

https://github.com/project-receptor/receptor/pull/36/files#diff-f311f62c5fdfdc737647797c7afac2beR3

Don't do this

@elyezer
Copy link
Copy Markdown
Member Author

elyezer commented Oct 30, 2019

@matburt I was adding the entire git repo and then removing afterwards but I get your concern. I've update the PR to add the whl generated by make dist and then installs from it.

Also I've removed the dnf update which is generating a smaller image. I can add it back in case anyone things that is important to get the most updated RPM packages.

@matburt
Copy link
Copy Markdown
Member

matburt commented Oct 30, 2019

We probably do need that DNF update... what's the size delta after that?

@elyezer
Copy link
Copy Markdown
Member Author

elyezer commented Oct 31, 2019

The image was updated recently (about an hour ago) and therefore there is no difference anymore, it was 80MB more before.

See below for the generated image size without the dnf update:

receptor             latest              b570cf745c20        12 seconds ago      255MB

And with the dnf update:

receptor             latest              66c36ab87aad        3 seconds ago       255MB

I'll update the PR with the dnf update in place.

Do the following:

* Use Fedora 30 as the base image
* Install receptor from source rather than wheel
* Set /var/lib/receptor as the home directory for receptor
@elyezer
Copy link
Copy Markdown
Member Author

elyezer commented Oct 31, 2019

Another change that I've made was to use the recent released Fedora 31 image and that is giving a smaller image:

receptor             latest              8bd93f5eca9c        About a minute ago   220MB

Copy link
Copy Markdown
Member

@matburt matburt left a comment

Choose a reason for hiding this comment

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

This looks good. We may still need to tweak the mechanism for getting the right dist into the image but this is good under a make clean && make image workflow.

@matburt matburt merged commit 0da9624 into project-receptor:master Nov 1, 2019
@elyezer elyezer deleted the update-dockerfile branch November 5, 2019 16:26
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