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

fixed host name for promtail config #2055

Merged
merged 11 commits into from
Sep 17, 2024
Merged

fixed host name for promtail config #2055

merged 11 commits into from
Sep 17, 2024

Conversation

pkrishnath
Copy link
Contributor

Why this change is needed

Fixed hostname env var for promtail config

Copy link
Collaborator

@BedrockSquirrel BedrockSquirrel left a comment

Choose a reason for hiding this comment

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

LGTM. I think the L1 and validator deploy scripts might not work properly with those matrix references though so can either fix that now or get it fixed replaced/later I guess (they're rarely used).

target_label: \"logstream\"
- source_labels: [\"__meta_docker_container_label_logging_jobname\"]
target_label: \"job\"
- replacement: ${{ matrix.host_id }}-${{ github.event.inputs.testnet_type }}-${{ GITHUB.RUN_NUMBER }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no 'matrix' in this script like there is in the L2 deploy script so maybe this will behave unpredictably. If it's just for keys to show how it will be rendered in Loki then maybe can be hardcoded to like L1-${{ github.event.inputs.testnet_type }} or something?

Not a blocker though, very rare we deploy the L1 for testnets and looks like someone already put in a matrix usage and it doesn't seem to be breaking so maybe it'll just do an empty string or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed : ${{ github.event.inputs.testnet_type }}-eth2network-${{ GITHUB.RUN_NUMBER }} as host ID

target_label: \"logstream\"
- source_labels: [\"__meta_docker_container_label_logging_jobname\"]
target_label: \"job\"
- replacement: ${{ matrix.host_id }}-${{ github.event.inputs.testnet_type }}-${{ GITHUB.RUN_NUMBER }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly because this validator deploys just a single node rather than the 'matrix' of 3 nodes deployed by the main deploy script so not sure what will happen if we try to run this.

But again not a blocker, rarely used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed: ${{ vars.AZURE_RESOURCE_PREFIX }}-${{ github.event.inputs.node_id }}-${{ GITHUB.RUN_NUMBER }}

@BedrockSquirrel
Copy link
Collaborator

Worth mentioning as well since we discussed separately, this is a stop-gap solution and all this stuff goes away in the new world with the CD tool doesn't it. But looks like this gets the automation of the agents working with our existing stuff nicely for now.

Copy link
Collaborator

@BedrockSquirrel BedrockSquirrel left a comment

Choose a reason for hiding this comment

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

Nice, LGTM

@pkrishnath
Copy link
Contributor Author

Replaced datadog agent with promtail in l1,l2 and validator dev-testnet workflows

@pkrishnath pkrishnath merged commit 0bb1220 into main Sep 17, 2024
2 checks passed
@pkrishnath pkrishnath deleted the promtail-agent branch September 17, 2024 09:31
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.

2 participants