Remove packages from registry Docker build #583
Conversation
|
Opening as draft as the change is not complete and it needs to be checked if this maybe effect the integrations repository. |
|
@mtojek I checked the integrations repo and it seems this will not effect it as in all places I could find, you mount your own volume and your own config with your own packages. Is this correct? |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
|
I also opened elastic/integrations#156 to attach integrations to a specific version of the registry. |
The Dockerfile for the registry itself should not contain any packages. Instead it should be empty and there are other distributions with packages, see elastic/package-storage#86. This removes the packages from the default Docker build. Few additional changes in the this PR: * Split up the Dockerfile in two stages to reduce the size of the image. Thanks @jsoriano for the contribution * Switch over to production packages instead of master * Select /packages as default path
|
Update the PR with all recommendations, should now be ready for a round of reviews. @jsoriano Your contribution will directly make our production images much smaller as they depend on this on here. |
|
|
||
| # Change to new working directory | ||
| WORKDIR /registry | ||
| WORKDIR /package-registry |
There was a problem hiding this comment.
Keep in mind that changing this path may break depending docker-compose definitions:
https://github.com/elastic/integrations/blob/master/testing/environments/snapshot.yml#L46
https://github.com/elastic/integrations/blob/master/testing/environments/kubernetes/snapshot.yml#L193
There was a problem hiding this comment.
Got it. Assuming we get elastic/integrations#156 in first, we could roll it out in two steps? I will update elastic/integrations#156 to also adjust the k8s one to a fix registry.
config.docker.yml
Outdated
| @@ -1,6 +1,6 @@ | |||
| public_dir: ./public | |||
| package_paths: | |||
| - ./packages/package-storage | |||
| - /packages | |||
There was a problem hiding this comment.
Let me explain why was there a reference to the package-storage.
In integrations we mount the second path for packages, so that we have:
./packages/package-storage
./packages/integrations
so that all packages are stored in the same /packages directory. Using a single directory prevents from simply loading another path with packages without replacing existing ones.
If you plan to change the current behavior here, I believe you need to update the testing/environment in integrations.
There was a problem hiding this comment.
My assumption was this keeps working as in https://github.com/elastic/integrations/blob/master/testing/environments/snapshot.yml#L47 you overwrite the default config?
There was a problem hiding this comment.
Updated to use /packages/package-registry as default and added a note to the dockerfile and config what users should do about it.
main_integration_test.go
Outdated
| @@ -23,6 +23,12 @@ import ( | |||
| // and the setup command works as expected. | |||
| func TestSetup(t *testing.T) { | |||
|
|
|||
| // Mage build is needed to pull in the packages from package-storage | |||
There was a problem hiding this comment.
I don't think this is correct.
test shouldn't build any code. That's why you've dependencies between targets (not sure if mage supports it) or you call it build test
There was a problem hiding this comment.
Updated to only use fetchPackageStorage instead to prevent the building of the binary.
There was a problem hiding this comment.
I'm afraid that this is something we won't skip.
There was a problem hiding this comment.
@mtojek Not sure I can follow your last comment. What part will we not skip?
|
For reference: #367 (comment) We talked about multi-stage built images a couple of weeks ago. I'm glad we have it finally! |
In elastic/package-registry#583 the package-registry was updated to not contain any packages anymore and the Dockerfile path for the config was adjusted. This updates the production registry accordingly.
In elastic/package-registry#583 the package-registry was updated to not contain any packages anymore and the Dockerfile path for the config was adjusted. This updates the production registry accordingly.
The Dockerfile for the registry itself should not contain any packages. Instead it should be empty and there are other distributions with packages, see elastic/package-storage#86. This removes the packages from the default Docker build.
Few additional changes in the this PR: