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

[networkcosts] Fix default loopback range to include all loopback... #1741

Merged
merged 1 commit into from
Oct 14, 2022

Conversation

LaikaN57
Copy link
Contributor

@LaikaN57 LaikaN57 commented Oct 13, 2022

… addresses

What does this PR change?

Updates the loopback address in the in-zone classification to include the entire loopback range as registered in IANA.

Authoritative Sources

IANA IPv4 Address Space Registry: https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml#:~:text=False-,127.0.0.0/8,-Loopback
IANA IPv4 Special-Purpose Address Registry: https://www.iana.org/assignments/ipv4-address-space/ipv4-address-space.xhtml#:~:text=ALLOCATED-,127/8,-IANA%20%2D%20Loopback

Does this PR rely on any other PRs?

No.

How does this PR impact users? (This is the kind of thing that goes in release notes!)

Some systems use other loopback addresses such as 127.0.0.53 (for some DNS resolvers), 127.1.0.1, etc. This change will now classify all addresses in the 127.0.0.0/8 range as in-zone by default.

Links to Issues or ZD tickets this PR addresses or fixes

N/A.

How was this PR tested?

Not tested. I would like some help testing this. I will also try to run a similar config in the next few days and update here once that is done.

Have you made an update to documentation?

Yes.

@@ -588,8 +588,8 @@ networkCosts:
# In Zone contains a list of address/range that will be
# classified as in zone.
in-zone:
# Loopback
- "127.0.0.1"
# Loopback Addresses in "IANA IPv4 Special-Purpose Address Registry"

Choose a reason for hiding this comment

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

To confirm, this would've incorrectly excluded this subset of loopback and would've appeared to have charged us?

Copy link

@corydolphin corydolphin Oct 13, 2022

Choose a reason for hiding this comment

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

Actually, how does this logic work? I see we have inzone specified, but not in-region, cross-region./

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To confirm, this would've incorrectly excluded this subset of loopback and would've appeared to have charged us?

Correct. It would have been excluding these addresses: 127.0.0.0 and 127.0.0.2 to 127.255.255.255.

@AjayTripathy
Copy link
Contributor

Thank you so much for this PR @LaikaN57 . This LGTM but I'd like @mbolt35 to take a final review.

Copy link
Contributor

@mbolt35 mbolt35 left a comment

Choose a reason for hiding this comment

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

This looks like a solid change to me!

@AjayTripathy AjayTripathy merged commit 5c43d0f into kubecost:develop Oct 14, 2022
AjayTripathy added a commit that referenced this pull request Oct 24, 2022
* update eks integration values to 1.97.0 (#1705)

* whitelist PV  used bytes

* Enable ContainerStats pipeline by default

Signed-off-by: Michael Dresser <michaelmdresser@gmail.com>

* fix: gcp workload identity (#1260)

* fix: gcp workload identity

Signed-off-by: Andrii Nasinnyk <anasinnyk@macpaw.com>

* network-costs serviceMonitor (#1710)

* network-costs serviceMonitor

* add nodeselectors for thanos pods (#1721)

* Wrap nested call

* removed unnecessary relabels

* comment

* Update values.yaml

* add ports to cost-analyzer deployment (#1733)

* [networkcosts] Fix default loopback range to include all loopback addresses (#1741)

* move kc deployment strategy configuration to values.yaml

* add default for null values entry

* port mappings (#1747)

* Add hostedDomain parameter

* Add a field for passing a soft memory limit for the go runtime

* Update values.yaml (#1678)

* Bump to v1.98.0-rc.1

Commit auto-generated by release script.

Signed-off-by: Michael Dresser (release bot variant) <michael@kubecost.com>

Signed-off-by: Michael Dresser <michaelmdresser@gmail.com>
Signed-off-by: Andrii Nasinnyk <anasinnyk@macpaw.com>
Signed-off-by: Michael Dresser (release bot variant) <michael@kubecost.com>
Co-authored-by: Linh Lam <108019527+linhlam-kc@users.noreply.github.com>
Co-authored-by: nickcurie <ncurie@kubecost.com>
Co-authored-by: Andrii Nasinnyk <andriy.nas+new@gmail.com>
Co-authored-by: Jesse Goodier <31039225+jessegoodier@users.noreply.github.com>
Co-authored-by: Bianca Burtoiu <bianca@kubecost.com>
Co-authored-by: Nick Curie <32180999+nickcurie@users.noreply.github.com>
Co-authored-by: Matt Bolt <mbolt35@gmail.com>
Co-authored-by: Alex Kennedy <alexzanderkennedy@gmail.com>
Co-authored-by: keith.hand <rkeithhand@gmail.com>
Co-authored-by: keith.hand <43491549+keithhand@users.noreply.github.com>
Co-authored-by: Webb Brown <298359+dwbrown2@users.noreply.github.com>
Co-authored-by: Ajay Tripathy <ajay@kubecost.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants