libvirt: Add Terraform variables for bootstrap node memory/CPU

[sjug]

Similar to #785, but for the bootstrap node.

[sjug]

Would be nice to have the terraform fmt also show -diff.

[wking]

The bootstrap node is done and gone before the heavy monitoring stuff rolls out. Is yours OOMing anyway?

[sjug]

@wking It's not OOMing, more so I'm just trying to give it more resources to speed things up a bit.

[sjug]

/test e2e-aws

[wking]

I'd expect we're network-limited by all the images we pull (especially if you aren't installing from a local registry). Can you post some benchmarks of time until bootstrap-complete on you system before and after this commit?

[abhinavdahiya]

I'd expect we're network-limited by all the images we pull (especially if you aren't installing from a local registry). Can you post some benchmarks of time until bootstrap-complete on you system before and after this commit?

+1 I don't think we should add knobs for bootstrap node. If we see that we need more memory lets just use that.

[abhinavdahiya]

@sjug I don't think we can accept this change of adding this tunable. But if you have some metrics that the current setup of bootstrap node is not adequate in terms of cpu / memory we can accept changing the defaults in this PR itself or close this one and open another with change to defaults.
Whatever you think is more appropriate. 😇

[sjug]

The install time is not very consistent so it was a bit challenging to measure any improvement or lack of. The times are an average of two runs, with large std dev.

As we increase cores:

2 cores	4 cores	6 cores
0:04:59	0:03:52	0:03:45

As we increase memory:

2G	4G	8G
0:04:59	0:04:45	0:04:10

That formatting was painful, apologies. Do we want to change the hardcoded default?

[crawford]

It looks like we should change the number of cores from two to four on the bootstrap domain. Is that valid for hosts which have less than four cores? Either way, I think we can close this PR in favor of changing the default.

[wking]

It looks like we should change the number of cores from two to four on the bootstrap domain.

Memory should have a reasonable cap, but won't CPUs just be "the more, the merrier"? I don't know if bootstrap CPU allocation is something we want to try to fine-tune.

Is that valid for hosts which have less than four cores?

Check by setting it to 256? If that works, maybe we should make 256 the default ;).

[cgwalters]

It looks like we should change the number of cores from two to four on the bootstrap domain.

Sanest thing is to match the host. On this topic, see https://pagure.io/standard-test-roles/pull-request/223

[mykaul]

What about the worker nodes, btw? Any thought on their sizing?

[wking]

What about the worker nodes, btw?

Worker nodes are created by the libvirt cluster-API provider, not by Terraform. So adjust their MachineSet objects just like you'd manage other Kubernetes resources (and ping the libvirt-provider maintainers if something isn't working ;).

[mykaul]

@wking - how do I fix the 1st worker though? It's created and overloaded with 93% of its memory consumed! I can ping them of course.

[wking]

how do I fix the 1st worker though...

We shouldn't distract here. Spin off to a libvirt-provider or separate installer issue? There may already be issues discussing this; I haven't checked

[sjug]

@cgwalters I'm not aware of any way to make terraform variables dynamic to get the host CPU count

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sjug
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: enxebre

If they are not already assigned, you can assign the PR to them by writing /assign @enxebre in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• data/data/libvirt/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sjug]

No longer have any variables for bootstrap node cpu or memory, closing.

[openshift-ci-robot]

@sjug: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-openstack	f82c0ca	link	/test e2e-openstack

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.