Added server alias to metrics #3620
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/assign @aledbf |
|
/cc @ElvinEfendi |
| @@ -130,6 +130,9 @@ func (n *NGINXController) syncIngress(interface{}) error { | |||
| for _, server := range servers { | |||
| if !hosts.Has(server.Hostname) { | |||
| hosts.Insert(server.Hostname) | |||
| if server.Alias != "" { | |||
| hosts.Insert(server.Alias) | |||
| } | |||
| } | |||
There was a problem hiding this comment.
This can result in duplicate server.Alias's in hosts collection. I think this if should be out of if !hosts.Has(server.Hostname) { and similar to that, like:
if !hosts.Has(server.Hostname) {
hosts.Insert(server.Hostname)
}
if server.Alias != "" && !hosts.Has(server.Alias) {
hosts.Insert(server.Alias)
}
There was a problem hiding this comment.
Hi, seems you missed this part.
The code still has if server.Alias != "" && !hosts.Has(server.Hostname) { instead of if server.Alias != "" && !hosts.Has(server.Alias) {
Currently, the alias would not be added to the hosts list because server.Hostname would be added in the previous line and !host.Has(server.Hostname) would always be false.
There was a problem hiding this comment.
@startnow65 thank you for noticing that error.
There was a problem hiding this comment.
Oops. Sorry about that.
I have never had to add a commit to a branch that was already merged.
I added the fix but I do not see the update here. Do I need to create a new PR?
There was a problem hiding this comment.
@walkafwalka no, I already fixed this in another PR.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
@walkafwalka thanks! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, walkafwalka The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
What this PR does / why we need it: Adds hosts specified via server alias to be added to them
Which issue this PR fixes fixes #3485
Special notes for your reviewer:
This PR is a resubmission of #3496.