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

IPAM utilization reporting #2011

Merged
merged 8 commits into from May 30, 2019

Conversation

Projects
None yet
5 participants
@neiljerram
Copy link
Member

commented May 23, 2019

Description

Todos

"github.com/projectcalico/calicoctl/calicoctl/commands/test_ipam"
)

func TestIPAM(args []string) error {

This comment has been minimized.

Copy link
@caseydavenport

caseydavenport May 23, 2019

Member

This is neat and seems useful to have.

however, I worry about adding it to calicoctl itself since users might not really understand it.

I think at a minimum we should have it be under some clearly named subcommand - e.g., calicoctl unsupported test-ipam or something like that.

Even better might just be to have a second tool that this repo builds that is only used for test purposes and not actually supplied to users.

And even better, I think my dream would be that instead of building a tool at all, we simply write the tests that require this in golang so they can just import the logic they need. WDYT?

@caseydavenport

This comment has been minimized.

Copy link
Member

commented May 23, 2019

I'm really excited about this!

A few quick thoughts on the output format

IP Pool 10.65.0.0/16 'ipam-test-v4'
    Block 10.65.79.0/26    59/64 IPs still available (92%)
IP Pool fd5f:a1b3:64::/48 'ipam-test-v6'
    Block fd5f:a1b3:64:4f2c:ec1b:27b9:1989:77c0/122    57/64 IPs still available (89%)
IP Pool 0.0.0.0/0 'orphaned allocation blocks'
    (no blocks)

I wonder if we should use a table format, similar to other calicoctl commands, with headings, columns, rows, etc. e.g,

POOL          CIDR                    UTILIZATION
my-pool       192.168.0.0/24          200/256 (78%)

Note the above also switches to counting the number of used IPs rather than the number of free IPs. Not sure which is better... it felt more intuitive to me that 100% would mean "full" rather than "empty".

This is different from the current format of calicoctl ipam show --ip, but I think is probably easier to read and more consistent with other commands.

We could then do a -o yaml or something to show the per-block stuff (though note that per-block isn't required for this first drop).

e.g.,

calicoctl ipam show my-pool -o yaml

- pool: my-pool
  cidr: 192.168.0.0/16
  blocks:
  - cidr: 192.168.0.0/26
    node: somenode
    usedIPs: 53

WDYT?

@fasaxc

This comment has been minimized.

Copy link
Member

commented May 28, 2019

Maybe s/UTILIZATION/USED/, that's a common idiom used by commands like df and free

              total        used        free      shared  buff/cache   available
Mem:       32568508     4424304    21417956      443144     6726248    27232104
Swap:       1003516           0     1003516
@neiljerram

This comment has been minimized.

Copy link
Member Author

commented May 28, 2019

@fasaxc @caseydavenport On the point about whether to report (primarily) the number in use, or the number available, I would personally be happy either way, but was swayed by the language in our customer request (RFE-137), which specifically asks for "how many IPs we still have available in a pool." Hence I broke the tie on that side. WDYT?

@caseydavenport

This comment has been minimized.

Copy link
Member

commented May 28, 2019

I like the idea of reporting both "USED" and "FREE", so the user can take their pick :)

@neiljerram neiljerram force-pushed the neiljerram:ipam-utilization branch from b29dad3 to 13fbbe0 May 29, 2019

@neiljerram neiljerram changed the title RFC - IPAM utilization IPAM utilization reporting May 29, 2019

@neiljerram

This comment has been minimized.

Copy link
Member Author

commented May 29, 2019

Note: needs glide update after projectcalico/libcalico-go#1087 has merged.

@neiljerram neiljerram force-pushed the neiljerram:ipam-utilization branch from 13fbbe0 to 453abd3 May 29, 2019

@neiljerram neiljerram requested a review from song-jiang May 29, 2019

@neiljerram

This comment has been minimized.

Copy link
Member Author

commented May 29, 2019

Sample output for calicoctl ipam show:

+----------+-------------------+------------+---------------+
| GROUPING |       CIDR        | IPS IN USE | IPS AVAILABLE |
+----------+-------------------+------------+---------------+
| IP Pool  | 10.65.0.0/16      | 5/64 (8%)  | 59/64 (92%)   |
| IP Pool  | fd5f:abcd:64::/48 | 7/64 (11%) | 57/64 (89%)   |
+----------+-------------------+------------+---------------+

and for calicoctl ipam show --show-blocks:

+----------+-------------------------------------------+------------+---------------+
| GROUPING |                   CIDR                    | IPS IN USE | IPS AVAILABLE |
+----------+-------------------------------------------+------------+---------------+
| IP Pool  | 10.65.0.0/16                              | 5/64 (8%)  | 59/64 (92%)   |
| Block    | 10.65.79.0/26                             | 5/64 (8%)  | 59/64 (92%)   |
| IP Pool  | fd5f:abcd:64::/48                         | 7/64 (11%) | 57/64 (89%)   |
| Block    | fd5f:abcd:64:4f2c:ec1b:27b9:1989:77c0/122 | 7/64 (11%) | 57/64 (89%)   |
+----------+-------------------------------------------+------------+---------------+

@neiljerram neiljerram referenced this pull request May 29, 2019

Merged

Doc for IPAM utilization reporting #2636

3 of 3 tasks complete

@neiljerram neiljerram force-pushed the neiljerram:ipam-utilization branch from a7defb2 to 2a3bec1 May 30, 2019

@song-jiang
Copy link
Member

left a comment

Looking good. I would suggest to add two more FV cases.

  • Test calicoctl ipam show --ip=10.240.0.300
  • Test an ippool with /30 CIDR and got all IPs allocated.

neiljerram added some commits May 30, 2019

Output format updates
1. Use potential pool capacity, not just sum of block capacity
2. Update format to IPS TOTAL, IPS IN USE, IPS FREE

(1) also means that we need to use floating point numbers, because IPv6
pool capacities may not fit in int or int64, and hence that we need to
choose format strings and precisions that look about right.
@song-jiang
Copy link
Member

left a comment

LGTM

@neiljerram neiljerram merged commit fccaa1c into projectcalico:master May 30, 2019

2 checks passed

license/cla Contributor License Agreement is signed.
Details
semaphoreci The build passed on Semaphore.
Details

@neiljerram neiljerram deleted the neiljerram:ipam-utilization branch May 30, 2019

@caseydavenport

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

Docs completed here, updating label: projectcalico/calico#2636 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.