Skip to content
This repository was archived by the owner on Mar 15, 2022. It is now read-only.

update to go 1.12#87

Merged
johnSchnake merged 3 commits into
vmware-archive:masterfrom
vigith:go12
Oct 2, 2019
Merged

update to go 1.12#87
johnSchnake merged 3 commits into
vmware-archive:masterfrom
vigith:go12

Conversation

@vigith

@vigith vigith commented Sep 26, 2019

Copy link
Copy Markdown
Contributor

Umbrella Issue #82

Go 1.12

@vigith vigith mentioned this pull request Sep 26, 2019
@vigith

vigith commented Sep 26, 2019

Copy link
Copy Markdown
Contributor Author

@johnSchnake Could you please review?

Comment thread Dockerfile Outdated
WORKDIR /app
RUN apk update --no-cache && apk add ca-certificates
COPY --from=build /src/eventrouter .
ADD eventrouter /eventrouter

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So something is wrong with the image that gets built.

I'm able to build it OK but then it fails to run:

$ kubectl logs eventrouter-854dbfdc6d-dt7lv -n kube-system
/bin/sh: /eventrouter: not found

When I run the image and look inside it is there:

$ pwd
/
$ ls -lah|grep event
-rwxr-xr-x    1 root     root       53.6M Sep 27 20:10 eventrouter

I think one issue is that you're putting it into the root directory but we are running as nobody:nobody, but even after I manually run the image as a root user I still get the same errors.

I tried moving it into /app/eventrouter as well and the same issues. I can see it in ls, I can even cat ./eventrouter|head and cp ./eventrouter foo but I can not run the files. I can chown them and chmod +x ... them as the root user, but still not run them.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I found the issue; see comment in makefile.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed the Makefile. I have also moved /eventrouter to /app/eventrouter

Comment thread Makefile Outdated

container:
$(DOCKER) build -t $(REGISTRY)/$(TARGET):latest --target release -t $(REGISTRY)/$(TARGET):$(VERSION) .
$(DOCKER_BUILD) 'go build'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need to be building the binary for 32-bit if we are going to be running it on alpine:3.9

Linux b789bf130d06 4.9.184-linuxkit #1 SMP Tue Jul 2 22:58:16 UTC 2019 x86_64 Linux

I think moving the binary into /app is also a good idea so its not in root, but then the binary needs to be the right architecture.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

adding CGO_ENABLED=0 to go build fixed the issue.

@johnSchnake johnSchnake left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes needed (see comments) in the makefile and dockerfile.

Ideally, you can rebase onto the tip of master and squash some of those commits (e.g. you have the multi-stage docker build then revert it as 2 of your commits)

Once that is done it should be good. My main test is seeing if I can run the basic yaml given as an example in the readme.

vigith added 3 commits October 1, 2019 13:19
Signed-off-by: Vigith Maurice <vigith@gmail.com>
Signed-off-by: Vigith Maurice <vigith@gmail.com>
update the Dockerfile and Makefile to support go12 and go mod
CGO_ENABLED=0 to remove dependency on glibc and move eventrouter to /app

Signed-off-by: Vigith Maurice <vigith@gmail.com>
@vigith

vigith commented Oct 1, 2019

Copy link
Copy Markdown
Contributor Author

I have made the changes in Makefile and Dockerfile. I have also tested it with the yaml file given in the readme session.

@johnSchnake

Copy link
Copy Markdown
Contributor

👍 Seems good to me. Thanks!

@johnSchnake johnSchnake merged commit 12218c3 into vmware-archive:master Oct 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants