-
Notifications
You must be signed in to change notification settings - Fork 39
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
Conversation
There was a problem hiding this 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 }} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 }} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 }}
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, LGTM
Replaced datadog agent with promtail in l1,l2 and validator dev-testnet workflows |
Why this change is needed
Fixed hostname env var for promtail config