Skip to content

Commit

Permalink
fix: current node hash can never be in the map
Browse files Browse the repository at this point in the history
Signed-off-by: Clément Nussbaumer <clement.nussbaumer@postfinance.ch>
  • Loading branch information
clementnuss committed Mar 11, 2024
1 parent 1689c1c commit 5753890
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 21 deletions.
7 changes: 5 additions & 2 deletions internal/kubenurse/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,11 @@ func New(ctx context.Context, c client.Client) (*Server, error) { //nolint:funle
chk.NeighbourFilter = os.Getenv("KUBENURSE_NEIGHBOUR_FILTER")
neighLimit := os.Getenv("KUBENURSE_NEIGHBOUR_LIMIT")

if chk.NeighbourLimit, err = strconv.Atoi(neighLimit); err != nil {
return nil, err
if neighLimit != "" {
chk.NeighbourLimit, err = strconv.Atoi(neighLimit)
if err != nil {
return nil, err
}
}

//nolint:goconst // No need to make "false" a constant in my opinion, readability is better like this.
Expand Down
21 changes: 10 additions & 11 deletions internal/servicecheck/neighbours.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

var osHostname = os.Hostname //nolint:gochecknoglobals // used during testing
//nolint:gochecknoglobals // used during testing
var (
osHostname = os.Hostname
currentNode string
)

// Neighbour represents a kubenurse which should be reachable
type Neighbour struct {
Expand Down Expand Up @@ -60,7 +64,8 @@ func (c *Checker) GetNeighbours(ctx context.Context, namespace, labelSelector st
continue
}

if pod.Name == hostname { // only quey other pods, not the currently running pod
if pod.Name == hostname { // only query other pods, not the currently running pod
currentNode = pod.Spec.NodeName
continue
}

Expand Down Expand Up @@ -108,19 +113,13 @@ func (c *Checker) filterNeighbours(nh []*Neighbour) []*Neighbour {

slices.Sort(l)

currentHostName, _ := osHostname()
hostnameHash := sha256String(currentHostName)

if m[hostnameHash].NodeName != currentHostName {
panic("the current hostname hash doesn't match the value in the map")
}

idx, _ := slices.BinarySearch(l, hostnameHash)
currentNodeHash := sha256String(currentNode)
idx, _ := slices.BinarySearch(l, currentNodeHash)

filteredNeighbours := make([]*Neighbour, 0, c.NeighbourLimit)

for i := 0; i < c.NeighbourLimit; i++ {
hash := l[(idx+i+1)%len(l)]
hash := l[(idx+i)%len(l)]
filteredNeighbours = append(filteredNeighbours, m[hash])
}

Expand Down
9 changes: 1 addition & 8 deletions internal/servicecheck/neighbours_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ func TestNodeFiltering(t *testing.T) {
nh := generateNeighbours(n)
require.NotNil(t, nh)

trueOsHostname := osHostname
defer func() { osHostname = trueOsHostname }()

// fake client, with a dummy neighbour pod
// fakeClient := fake.NewFakeClient(&fakeNeighbourPod)
checker := Checker{
NeighbourLimit: neighbourLimit,
}
Expand All @@ -43,9 +38,7 @@ func TestNodeFiltering(t *testing.T) {
counter := make(map[string]int, n)

for i := range n {
osHostname = func() (name string, err error) {
return nh[i].NodeName, nil
}
currentNode = nh[i].NodeName
filtered := checker.filterNeighbours(nh)
require.Equal(t, neighbourLimit, len(filtered))

Expand Down

0 comments on commit 5753890

Please sign in to comment.