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

libvirt: 1st worker needs more memory #1041

Closed
mykaul opened this issue Jan 10, 2019 · 6 comments
Closed

libvirt: 1st worker needs more memory #1041

mykaul opened this issue Jan 10, 2019 · 6 comments

Comments

@mykaul
Copy link
Contributor

mykaul commented Jan 10, 2019

I suspect (perhaps because of https://github.com/openshift/cluster-api-provider-libvirt/blob/da6aa0a3931761842527816a79f1072129f2cea3/test/utils/manifests.go#L67 ) that it copies the same values as the master default, and 2GB are simply not enough for it. It is using 84% of its memory if the installation succeeded.

(Is that a cluster-api-provider-libvirt issue?)

It'd be great if we could configure its properties.

@wking
Copy link
Member

wking commented Jan 10, 2019

It is using 84% of its memory if the installation succeeded.

You have a few options here:

a. Configuring larger workers. We don't have a knob for this at the install-config level, but you can use a multi-step install with a stop at openshift-install --dir=whatever create manifests to edit the provider in whatever/openshift/99_openshift-cluster-api_worker-machineset.yaml.
b. You can launch the cluster with the default worker MachineSet(s), and then update those objects to add more replicas once they've been pushed (any time after the bootstrap-complete event would be fine). You may be able to change provider fields to allocate more memory, but I dunno if the libvirt provider supports that yet (although it probably would if you scaled replicas down to zero, changed whatever, and then scaled back it).
c. You can launch the cluster with the default worker MachineSet(s), and then push additional Machine(Set)(s) to allocate additional nodes.

(Is that a cluster-api-provider-libvirt issue?)

(b) and (c) are purely day-2, libvirt-provider issues. (a) is purely an installer issue.

Also in this space is the fact that almost everything launched by the cluster-version operator (directly and indirectly) should tolerate masters, although there are still a handful of operators and such which don't. That's an issue with those operators, and once they get patched up, you'll be able to relieve worker contention by provisioning more master space (e.g. via #785).

@mykaul
Copy link
Contributor Author

mykaul commented Jan 10, 2019

I also found that apparently this works:

[ykaul@ykaul installer]$ git diff
diff --git a/pkg/asset/machines/libvirt/machines.go b/pkg/asset/machines/libvirt/machines.go
index 9d3ccf66f..441e61501 100644
--- a/pkg/asset/machines/libvirt/machines.go
+++ b/pkg/asset/machines/libvirt/machines.go
@@ -67,7 +67,7 @@ func provider(clusterName string, networkInterfaceAddress string, platform *libv
                        APIVersion: "libvirtproviderconfig.k8s.io/v1alpha1",
                        Kind:       "LibvirtMachineProviderConfig",
                },
-               DomainMemory: 2048,
+               DomainMemory: 4096,
                DomainVcpu:   2,
                Ignition: &libvirtprovider.Ignition{
                        UserDataSecret: userDataSecret,

Regretfully, I'm again unsuccessful at installing the whole thing end to end, but I doubt it's related.

I personally think we should go for solution A. The 1st worker is just overloaded with stuff and has to be bigger. Note that like every qemu VM, it'll consume as much memory as it needs from the host, so if it only uses 3GB, it'll use them and only them, not 4GB, for example.

@wking wking changed the title (libvirt) 1st worker needs more memory libvirt: 1st worker needs more memory Jan 10, 2019
@wking
Copy link
Member

wking commented Jan 10, 2019

I personally think we should go for solution A...

Everyone can pick whichever of these options they like best. There's no need for installer source changes or otherwise forming a consensus.

@mykaul
Copy link
Contributor Author

mykaul commented Feb 21, 2019

@wking - seems this can be closed, as my idea (in #1041 (comment) ) was just implemented in db5f0ae ?

(I still can't complete an installation successfully, but at least it has enough memory!)

@wking
Copy link
Member

wking commented Feb 21, 2019

Thanks :)

@wking wking closed this as completed Feb 21, 2019
@deanpeterson
Copy link

I personally think we should go for solution A...

Everyone can pick whichever of these options they like best. There's no need for installer source changes or otherwise forming a consensus.

Solution A has no effect. The changes made are not honored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants