Skip to content

Conversation

@huntergregory
Copy link
Contributor

@huntergregory huntergregory commented Jun 5, 2023

Reason for Change:
Windows has scale limits in terms of total Pod IPs in the cluster (on either Windows or Linux nodes).

Issue Fixed:

Requirements:

Notes:
Also removes unnecessary metric for max ipset members. Total Pod IPs is a more accurate/relevant metric.

Metric Preview

# HELP npm_pods_watched Number of Pods NPM tracks across the cluster including Linux and Windows nodes
# TYPE npm_pods_watched gauge
npm_pods_watched 22

@huntergregory huntergregory requested a review from vakalapa June 5, 2023 23:39
@huntergregory huntergregory requested a review from a team as a code owner June 5, 2023 23:39
// Create npmPod and add it to the podMap
npmPodObj := common.NewNpmPod(podObj)
c.podMap[podKey] = npmPodObj
metrics.AddPod()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Goal: call metrics.AddPod() once per Pod/IP pair (similar for metrics.RemovePod())

  • If a Pod has an update to its IP, follow Pod DELETE then Pod CREATE.
  • For Pod DELETE: c.cleanupDeletePod() is only called when podKey exists in c.podMap
  • For Pod CREATE: c.syncAddedPod() is only called when podKey does not exist in c.podMap. Otherwise, another codepath is taken.
  • Keys are only added/removed from c.podMap in c.syncAddedPod() and c.cleanupDeletePod()

@huntergregory huntergregory force-pushed the hgregory/06-05-ip-metric branch from 8b1478b to df361fc Compare June 6, 2023 00:28
@huntergregory huntergregory enabled auto-merge (squash) June 7, 2023 20:22
@vakalapa vakalapa disabled auto-merge June 9, 2023 16:59
@vakalapa vakalapa merged commit e6eeb50 into master Jun 9, 2023
@vakalapa vakalapa deleted the hgregory/06-05-ip-metric branch June 9, 2023 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

linux npm Related to NPM. windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants