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

Monitoring #103

Merged
merged 36 commits into from
Oct 4, 2022
Merged

Monitoring #103

merged 36 commits into from
Oct 4, 2022

Conversation

topliceanu
Copy link
Contributor

No description provided.

@topliceanu topliceanu mentioned this pull request Sep 5, 2022
@topliceanu topliceanu temporarily deployed to integration September 5, 2022 14:34 Inactive
@github-actions
Copy link

github-actions bot commented Sep 5, 2022

Smoke Test Results

1 tests   1 ✔️  7m 2s ⏱️
1 suites  0 💤
1 files    0

Results for commit 9126230.

♻️ This comment has been updated with latest results.


# Production image

FROM ubuntu:20.04
Copy link

Choose a reason for hiding this comment

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

I would recommend alpine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I added a TODO to update this at a later point. It shouldn't require any other changes. I'll use this https://github.com/smartcontractkit/atlas/blob/master/evm/Dockerfile as an example. Wdyt?

Copy link
Collaborator

@archseer archseer Sep 13, 2022

Choose a reason for hiding this comment

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

I wonder if you need alpine and scratch could be enough?

As long as you

# copy ca certs
COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/
 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do this in a separate PR. Wdyt?

relayer/pkg/chainlink/ocr2/client.go Show resolved Hide resolved
relayer/pkg/chainlink/ocr2/types.go Show resolved Hide resolved
relayer/pkg/monitoring/config_chain.go Outdated Show resolved Hide resolved
relayer/pkg/monitoring/source_envelope.go Outdated Show resolved Hide resolved
@topliceanu topliceanu temporarily deployed to integration September 12, 2022 13:00 Inactive
@topliceanu topliceanu temporarily deployed to integration September 12, 2022 13:09 Inactive
Comment on lines 3 to 23
FROM ubuntu:20.04 AS build

# OS dependencies
RUN apt-get update && apt-get install -y wget gcc build-essential git

# Copy source

RUN mkdir -p /starknet-monitoring/relayer
COPY ./relayer /starknet-monitoring/relayer

# Install golang

RUN wget -c https://dl.google.com/go/go1.18.1.linux-amd64.tar.gz -O - \
| tar -xz -C /usr/local \
&& mkdir -p /go/src /go/bin
ENV PATH /usr/local/go/bin:$PATH

# Compile binary

WORKDIR /starknet-monitoring/relayer
RUN go build -o ./monitoring ./cmd/monitoring/*.go
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since all this is doing is just compiling go code, why not use the golang build image like core does?

https://github.com/smartcontractkit/chainlink/blob/b64fe315a2e7af10244f737c9acd7bd3c75bbeef/core/chainlink.Dockerfile#L30

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do this in a separate PR. Wdyt?

@topliceanu
Copy link
Contributor Author

@archseer you marked your review as "requesting changes". What is exactly that you want me to change? the Dockerfile?

@archseer
Copy link
Collaborator

archseer commented Sep 16, 2022 via email

relayer/ops/monitoring/Dockerfile Outdated Show resolved Hide resolved
relayer/go.mod Outdated Show resolved Hide resolved
relayer/pkg/chainlink/ocr2/client.go Outdated Show resolved Hide resolved
relayer/pkg/monitoring/config_feed.go Outdated Show resolved Hide resolved
@topliceanu topliceanu temporarily deployed to integration September 30, 2022 13:50 Inactive
@topliceanu topliceanu temporarily deployed to integration September 30, 2022 13:56 Inactive
Copy link
Collaborator

@aalu1418 aalu1418 left a comment

Choose a reason for hiding this comment

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

looking good! just small things :)

monitoring/go.mod Outdated Show resolved Hide resolved
relayer/pkg/chainlink/ocr2/types_test.go Outdated Show resolved Hide resolved
relayer/pkg/chainlink/ocr2/types_test.go Outdated Show resolved Hide resolved
monitoring/ops/Dockerfile Show resolved Hide resolved
@topliceanu topliceanu temporarily deployed to integration October 3, 2022 16:15 Inactive
@topliceanu topliceanu temporarily deployed to integration October 3, 2022 16:26 Inactive
@topliceanu topliceanu temporarily deployed to integration October 3, 2022 16:31 Inactive
Copy link
Collaborator

@aalu1418 aalu1418 left a comment

Choose a reason for hiding this comment

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

looks good to me! happy to approve knowing there will be follow up PRs :)

NOTE: this PR should not be squashed - commit history needs to be preserved

@topliceanu topliceanu dismissed archseer’s stale review October 4, 2022 08:55

"Dismiss" is a strong word! I'll update the Dockerfile in a separate PR.

@topliceanu topliceanu merged commit 471b1fb into develop Oct 4, 2022
@topliceanu topliceanu deleted the monitoring-v2 branch October 4, 2022 08:55
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.

5 participants