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

Remove packages from registry Docker build #583

Merged
merged 4 commits into from
Jul 3, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,3 @@ package-registry

.idea
build
public/*
49 changes: 21 additions & 28 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,44 +1,37 @@
# This Dockerfile allows to build the package-registry and packages together.
# It is decoupled from the packages and the registry even though for now both are in the same repository.
# This image contains the package-registry binary.
# It expects packages to be mounted under /packages or have a config file loaded into /package-registry/config.yml

# Build binary
ARG GO_VERSION=1.14.2
FROM golang:${GO_VERSION}
FROM golang:${GO_VERSION} AS builder

# Get dependencies
RUN \
apt-get update \
&& apt-get install -y --no-install-recommends zip rsync \
&& rm -rf /var/lib/apt/lists/*
ENV GO111MODULE=on
COPY ./ /package-registry
WORKDIR /package-registry
RUN go build .

# Copy the package registry
COPY ./ /home/package-registry
WORKDIR /home/package-registry

ENV GO111MODULE=on
RUN go get -u github.com/magefile/mage
# Prepare all the packages to be built
RUN mage build
# Run binary
FROM centos:7

# Build binary
RUN go build .
# Get dependencies
RUN yum install -y zip rsync && yum clean all

# Move all files need to run to its own directory
# This will become useful for staged builds later on
RUN mkdir -p /registry/public # left for legacy purposes
RUN mkdir -p /registry/packages/package-storage
RUN mv package-registry /registry/
RUN cp -r build/package-storage/packages/* /registry/packages/package-storage/
RUN cp config.docker.yml /registry/config.yml
# Move binary from the builder image
COPY --from=builder /package-registry/package-registry /package-registry/package-registry

# Change to new working directory
WORKDIR /registry
WORKDIR /package-registry
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


# Clean up files not needed
RUN rm -rf /home/package-registry
# Get in config which expects packages in /packages
COPY config.docker.yml /package-registry/config.yml

# Start registry when container is run an expose it on port 8080
EXPOSE 8080

ENTRYPOINT ["./package-registry"]

# Make sure it's accessible from outside the container
CMD ["--address=0.0.0.0:8080"]

HEALTHCHECK --interval=1s --retries=30 CMD curl --silent --fail localhost:8080/health || exit 1

2 changes: 1 addition & 1 deletion config.docker.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
public_dir: ./public
package_paths:
- ./packages/package-storage
- /packages
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use /packages/package-registry as default and added a note to the dockerfile and config what users should do about it.


cache_time.search: 10m
cache_time.categories: 10m
Expand Down
2 changes: 1 addition & 1 deletion magefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func fetchPackageStorage() error {

packageStorageRevision := os.Getenv("PACKAGE_STORAGE_REVISION")
if packageStorageRevision == "" {
packageStorageRevision = "master"
packageStorageRevision = "production"
}

err := sh.Run("git",
Expand Down
6 changes: 6 additions & 0 deletions main_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to only use fetchPackageStorage instead to prevent the building of the binary.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid that this is something we won't skip.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mtojek Not sure I can follow your last comment. What part will we not skip?

Copy link
Contributor

Choose a reason for hiding this comment

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

not to fetch packages

err := sh.Run("mage", "build")
if err != nil {
t.Error(err)
}

currentDir, err := os.Getwd()
if err != nil {
t.Error(err)
Expand Down
4 changes: 3 additions & 1 deletion testing/environments/local.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ services:

package-registry:
build: ../..
image: docker.elastic.co/package-registry/package-registry:master
#image: docker.elastic.co/package-registry/package-registry:master
volumes:
- ../../build/package-storage/packages:/packages
ports:
- "127.0.0.1:8080:8080"