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

Display hostName other than podName #118

Merged
merged 2 commits into from
Apr 14, 2022

Conversation

j4ckstraw
Copy link
Contributor

Issue number of the reported bug or feature request: #

Describe your changes
I changed pod.Name to p.Spec.NodeName, so we can see the nodename other than podname in the UI.
image

Testing performed
Describe the testing you have performed to ensure that the bug has been addressed, or that the new feature works as planned.

Additional context
Add any other context about your contribution here.
goldpinger is deployed by daemonset, so nodename is better than podname.

Signed-off-by: wanglijie6 <wanglijie6@xiaomi.com>
@seeker89
Copy link
Contributor

Hi @j4ckstraw thanks for the PR!

How would you feel about making your behaviour change hide behind an extra command line flag, that's defaulting to false for backwards compatibility?

I see how this might be useful, but we don't want to be break existing users.

@j4ckstraw
Copy link
Contributor Author

OK, I will file the patch later.

Add GoldpingerConfig.DisplayNodeName to control UI display, default is
`false`, which means to display podName

Signed-off-by: wanglijie6 <wanglijie6@xiaomi.com>
@j4ckstraw
Copy link
Contributor Author

fix #99

Copy link
Contributor

@seeker89 seeker89 left a comment

Choose a reason for hiding this comment

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

🎉 sounds good, LGTM!

@seeker89 seeker89 merged commit a461b0f into bloomberg:master Apr 14, 2022
@j4ckstraw j4ckstraw deleted the feat/show-nodename branch April 15, 2022 09:37
@wesleyoliveiraleite
Copy link

Hi, how can I use the env "DISPLAY_NODENAME" in my daemonset yaml file? This was my trying, but not working:

- name: DISPLAY_NODENAME
  valueFrom:
    fieldRef:
      fieldPath: spec.nodeName

I'd like to have the view in UI goldpinger the node name instead pod name.

@tamcore
Copy link

tamcore commented Sep 29, 2022

@wesleyoliveiraleite have you tried the following?

- name: DISPLAY_NODENAME
  value: "true"

Because the parameter is a boolean.

@seeker89
Copy link
Contributor

seeker89 commented Oct 5, 2022

Yup, @tamcore is right. Thanks!

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.

None yet

4 participants