Skip to content

Conversation

@mplsgrant
Copy link
Collaborator

This fixes the logs container selection issues in #623

@mplsgrant mplsgrant requested a review from pinheadmz October 1, 2024 22:10
try:
pod = get_pod(tank)
container_names = [container.name for container in pod.spec.containers]
container_name = container_names[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

What determines the order of containers in a pod? Can we guarantee the one we want is always [0] because of the yaml file? Or should we hard code the name from the chart bitcoincore ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The first container has been the bitcoincore container and commander container from what I have seen, but I agree that we have no way of guaranteeing that.

I changed the logic so that we use hard coded values from the constants file which match the helm charts.

@pinheadmz
Copy link
Contributor

looks good! testing ...

Copy link
Contributor

@pinheadmz pinheadmz left a comment

Choose a reason for hiding this comment

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

👍 ACK

@mplsgrant
Copy link
Collaborator Author

Thanks @pinheadmz I'll merge

@mplsgrant mplsgrant merged commit d2e277d into bitcoin-dev-project:main Oct 2, 2024
10 checks passed
@mplsgrant mplsgrant deleted the 2024-10-fix-logs-container branch October 2, 2024 18:34
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