Skip to content

Metrics: add ipv4/6 addresses_in_use_total and addresses_total - #2151

Merged
fedepaol merged 2 commits into
metallb:mainfrom
woodgear:feat/dualstack-ip-metrics
Jan 16, 2024
Merged

Metrics: add ipv4/6 addresses_in_use_total and addresses_total#2151
fedepaol merged 2 commits into
metallb:mainfrom
woodgear:feat/dualstack-ip-metrics

Conversation

@woodgear

@woodgear woodgear commented Nov 8, 2023

Copy link
Copy Markdown
Contributor

issue: #2150

@woodgear woodgear mentioned this pull request Nov 8, 2023
Closed
2 tasks
@woodgear woodgear changed the title demo impl of add ipv4/ipv6 PoolActive and PoolCapacity in metrics draft: demo impl of add ipv4/ipv6 PoolActive and PoolCapacity in metrics Nov 8, 2023
@woodgear
woodgear marked this pull request as draft November 8, 2023 05:12
@woodgear woodgear changed the title draft: demo impl of add ipv4/ipv6 PoolActive and PoolCapacity in metrics demo impl of add ipv4/ipv6 PoolActive and PoolCapacity in metrics Nov 8, 2023
@woodgear
woodgear force-pushed the feat/dualstack-ip-metrics branch from 55c30a8 to e3fc2ce Compare November 8, 2023 10:44
@fedepaol

fedepaol commented Nov 30, 2023

Copy link
Copy Markdown
Member

Sorry for the delay. I think this makes sense, and would even align with https://github.com/metallb/metallb/blob/main/design/crd-status.md

WDYT @oribon ?

@oribon

oribon commented Nov 30, 2023

Copy link
Copy Markdown
Member

sounds good

@woodgear

woodgear commented Dec 2, 2023

Copy link
Copy Markdown
Contributor Author

cool。 i’will start working on this。cr status and metrics are different。 like prometheus could not monitor via crd status，so i thought we should add it both。in this pr，i will still focus on the metrics part。

@fedepaol

fedepaol commented Dec 12, 2023

Copy link
Copy Markdown
Member

cool。 i’will start working on this。cr status and metrics are different。 like prometheus could not monitor via crd status，so i thought we should add it both。in this pr，i will still focus on the metrics part。

Yep it's totally fine to work only on the metrics part.

@fedepaol

fedepaol commented Jan 9, 2024

Copy link
Copy Markdown
Member

Hey @woodgear , just pinging to check if you are still interested to work on this

@woodgear
woodgear force-pushed the feat/dualstack-ip-metrics branch 4 times, most recently from 9bcd362 to 2acf07b Compare January 12, 2024 03:06
@woodgear

woodgear commented Jan 12, 2024

Copy link
Copy Markdown
Contributor Author

yes. im try to fighting the ci..

@woodgear
woodgear marked this pull request as ready for review January 12, 2024 03:25
@woodgear
woodgear force-pushed the feat/dualstack-ip-metrics branch from 2acf07b to 615ce80 Compare January 12, 2024 03:42
@woodgear woodgear changed the title demo impl of add ipv4/ipv6 PoolActive and PoolCapacity in metrics Metrics: add ipv4/6 addresses_in_use_total and addresses_total Jan 12, 2024
@woodgear

woodgear commented Jan 12, 2024

Copy link
Copy Markdown
Contributor Author

@fedepaol

fedepaol
fedepaol reviewed Jan 12, 2024
View reviewed changes
Comment thread internal/allocator/allocator.go
Comment thread internal/k8s/controllers/reconciliation_test.go
Comment thread e2etest/l2tests/l2.go Outdated
@fedepaol

fedepaol commented Jan 12, 2024

Copy link
Copy Markdown
Member

Thanks, a couple of nits but looks really neat!

@woodgear
woodgear force-pushed the feat/dualstack-ip-metrics branch 4 times, most recently from fe98190 to a1df7f7 Compare January 12, 2024 11:39
fedepaol
fedepaol reviewed Jan 12, 2024
View reviewed changes
Comment thread e2etest/l2tests/l2.go Outdated
@woodgear
woodgear force-pushed the feat/dualstack-ip-metrics branch 2 times, most recently from 2c42866 to d31790b Compare January 12, 2024 14:28
@woodgear
Metrics: add ipv4/6 addresses_in_use_total and addresses_total
9cb41da 
Signed-off-by: cong <q1875486458@gmail.com>
@woodgear
woodgear force-pushed the feat/dualstack-ip-metrics branch from 0a46e1c to 9cb41da Compare January 12, 2024 14:32
@woodgear

woodgear commented Jan 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@fedepaol updated, please take a look and merge?

@fedepaol

fedepaol commented Jan 15, 2024

Copy link
Copy Markdown
Member

LGTM, one final ask: #2151 (comment)
Can you move that change in a separate commit (still in this PR)? Thanks a lot!

@woodgear
lint: fix for golangci-lint
00e6e1a 
Signed-off-by: cong <q1875486458@gmail.com>
@woodgear

woodgear commented Jan 16, 2024

Copy link
Copy Markdown
Contributor Author

@fedepaol updated.

@fedepaol

fedepaol commented Jan 16, 2024

Copy link
Copy Markdown
Member

Awesome, sending to the merge queue!

@fedepaol
fedepaol enabled auto-merge January 16, 2024 08:28
@fedepaol
fedepaol added this pull request to the merge queue Jan 16, 2024
Merged via the queue into metallb:main with commit b0d17fc Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fedepaol fedepaol fedepaol left review comments
@russellb russellb Awaiting requested review from russellb
@gclawes gclawes Awaiting requested review from gclawes
@oribon oribon Awaiting requested review from oribon oribon is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@woodgear @fedepaol @oribon