This commit replaces `os.Setenv` with `t.Setenv` in tests. The
environment variable is automatically restored to its original value
when the test and all its subtests complete.
Reference: https://pkg.go.dev/testing#T.Setenv
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
This commit is a combination of several commits. Significant details are
preserved below.
* update functions for resource annotations
This change converts some of the functions that look at annotation for
resource usage to indicate their usage in the function name. This helps
to make room for allowing the infrastructure reference as an alternate
source for the capacity information.
* migrate capacity logic into a single function
This change moves the logic to collect the instance capacity from the
TemplateNodeInfo function into a method of the
unstructuredScalableResource named InstanceCapacity. This new function
is created to house the logic that will decide between annotations and
the infrastructure reference when calculating the capacity for the node.
* add ability to lookup infrastructure references
This change supplements the annotation lookups by adding the logic to
read the infrastructure reference if it exists. This is done to
determine if the machine template exposes a capacity field in its
status. For more information on how this mechanism works, please see the
cluster-api enhancement[0].
* add documentation for capi scaling from zero
* improve tests for clusterapi scale from zero
this change adds functionality to test the dynamic client behavior of
getting the infrastructure machine templates.
* update README with information about rbac changes
this adds more information about the rbac changes necessary for the
scale from zero support to work.
* remove extra check for scaling from zero
since the CanScaleFromZero function checks to see if both CPU and
memory are present, there is no need to check a second time. This also
adds some documentation to the CanScaleFromZero function to make it
clearer what is happening.
* update unit test for capi scale from zero
adding a few more cases and details to the scale from zero unit tests,
including ensuring that the int based annotations do not accept other
unit types.
[0] https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20210310-opt-in-autoscaling-from-zero.md
This allows a Machine{Set,Deployment} to scale up/down from 0,
providing the following annotations are set:
```yaml
apiVersion: v1
items:
- apiVersion: machine.openshift.io/v1beta1
kind: MachineSet
metadata:
annotations:
machine.openshift.io/cluster-api-autoscaler-node-group-min-size: "0"
machine.openshift.io/cluster-api-autoscaler-node-group-max-size: "6"
machine.openshift.io/vCPU: "2"
machine.openshift.io/memoryMb: 8G
machine.openshift.io/GPU: "1"
machine.openshift.io/maxPods: "100"
```
Note that `machine.openshift.io/GPU` and `machine.openshift.io/maxPods`
are optional.
For autoscaling from zero, the autoscaler should convert the mem value
received in the appropriate annotation to bytes using powers of two
consistently with other providers and fail if the format received is not
expected. This gives robust behaviour consistent with cloud providers APIs
and providers implementations.
https://cloud.google.com/compute/all-pricinghttps://www.iec.ch/si/binary.htmhttps://github.com/openshift/kubernetes-autoscaler/blob/master/cluster-autoscaler/cloudprovider/aws/aws_manager.go#L366
Co-authored-by: Enxebre <alberto.garcial@hotmail.com>
Co-authored-by: Joel Speed <joel.speed@hotmail.co.uk>
Co-authored-by: Michael McCune <elmiko@redhat.com>
This change updates the logic for the clusterapi autoscaler provider so
that the `CAPI_GROUP` environment variable will also affect the
annotations keys for minimum and maximum node group size, the machine
annotation, machine deletion, and the cluster name label. It also addes
unit tests and an update to the readme.
We index on providerID but it turns out that those values on node and
machine are not always consistent. Some encode region, some do not,
for example.
This commit normalizes all values through the normalizedProviderString().
To ensure that we catch all places I've introduced a new type and made
the find() functions take this new type in lieu of a string. Unit
tests have also been adjusted to introduce a 'test:///' prefix on the
providerID value to further validate the change.
This change allows CAPI to work out-of-the-box, assuming v1alpha2.
It's also reasonable to assert that this consistency should be
enforced elsewhere and to make this behaviour easily revertable I'm
leaving this as a separate commit in this patch series.
Eng Zer Jun
Michael McCune
Andrew McDermott
enxebre
Jason DeTiberus
Jason DeTiberus