Ingester binaries & Docker #1086
Conversation
ledor473
requested review from
black-adder,
jpkrohling,
pavolloffay,
vprithvi and
yurishkuro
as code owners
Makefile
Outdated
| build-ingester: | ||
| CGO_ENABLED=0 installsuffix=cgo go build -o ./cmd/ingester/ingester-$(GOOS) $(BUILD_INFO) ./cmd/ingester/main.go | ||
|
|
||
| .PHONY: build-ingester-linux |
There was a problem hiding this comment.
This was added by @zoidbergwill but there's no build-collector-linux nor a build-agent-linux, so I see 2 options:
- I remove this and we recommend using
GOOS=linux make build-ingester - I add one for every binary
There was a problem hiding this comment.
Ahhh, I see now, only the all in one has a -linux job. Recommending GOOS=linux make build-ingester would definitely be good enough. I thought I'd found some other ones that did the same, my bad.
There was a problem hiding this comment.
Hum I missed the all-in-one one! Not sure why we have a specific step for it
There was a problem hiding this comment.
don't think we need build-ingester-linux. Not sure why all-in-one needs it either.
There was a problem hiding this comment.
It's used by ./scripts/travis/build-all-in-one-image.sh, but I can add that cleanup to the PR
ledor473
force-pushed
the
ingester-binaries
branch
from
c444ff0
to
e04b4e9
Compare
|
DCO complains about the commit from @zoidbergwill so I'm not sure how to handle that.
|
|
Whoops, those are both me. What's the easiest way to fix it? Could I rebase that commit and fix my sign-off / author e-mail? |
Codecov Report
@@ Coverage Diff @@
## master #1086 +/- ##
======================================
Coverage 100% 100%
======================================
Files 140 140
Lines 6622 6622
======================================
Hits 6622 6622Continue to review full report at Codecov.
|
ledor473
force-pushed
the
ingester-binaries
branch
from
e04b4e9
to
e7635b3
Compare
|
Thanks @zoidbergwill 👍 I've rebase my change and DCO is ✅ now. |
+1 |
|
futurama refs are 1st class citizen in this repo |
|
|
||
| FROM scratch | ||
|
|
||
| COPY --from=certs /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt |
There was a problem hiding this comment.
at this point prob not, but I assume it's feasible that people might use certs to auth to Kafka.
@ledor473 maybe forgot to push? |
ledor473
force-pushed
the
ingester-binaries
branch
from
e139ccf
to
b18adae
Compare
|
Seems like I forgot to sign my 2nd commit indeed. It's fixed now |
|
should this be rebased or merge master in? 'cause there's a fix for Kafka tests in master. |
Signed-off-by: William Stewart <zoidbergwill@gmail.com>
Signed-off-by: Louis-Etienne Dorval <louis-etienne.dorval@ticketmaster.com>
ledor473
force-pushed
the
ingester-binaries
branch
from
b18adae
to
2ed6798
Compare
|
Rebased, squashed and confirmed that I've signed my commit 👍 |
|
Awesome! |
Which problem is this PR solving?
Resolves #1031
Short description of the changes
This supersede #1033