diff --git a/README.md b/README.md index 4d33277..4cb77c2 100644 --- a/README.md +++ b/README.md @@ -149,9 +149,59 @@ Provisioned nodes carry, in addition to the well-known Karpenter labels: | `labels` | `map[string]string` | no | — | Extra hcloud labels on the Hetzner server (useful for cost attribution or firewall label-selectors) | | `userData` | `string` | no | — | Inline cloud-init / Talos machine config. Overridden by `userDataSecretRef` when both are set. | | `userDataSecretRef` | `object {namespace, name, key}` | no | — | Source `userData` from a Secret instead of inline. The Secret is read at server-create time; its value never appears in the NodeClass spec or git. Takes precedence over `userData`. | +| `kubelet.systemReserved` | `map[string]string` | no | — | What your bootstrap passes to the kubelet as `--system-reserved`. Keys: `cpu`, `memory`, `ephemeral-storage`, `pid`. | +| `kubelet.kubeReserved` | `map[string]string` | no | — | What your bootstrap passes as `--kube-reserved`. Same keys. | +| `kubelet.evictionHard` | `map[string]string` | no | `memory.available: 100Mi`, `nodefs.available: 10%` | What your bootstrap passes as `--eviction-hard`. Values are a quantity (`400Mi`) or a percentage (`10%`). | Status exposes `conditions` (`ImagesReady`, `NetworkReady`, `ResourcesReady`, `UserDataReady`, aggregated into `Ready`) and `resolvedImages` (image ID per architecture). +### Declaring kubelet reservations + +Karpenter decides which server type a pod fits on by subtracting reserved +resources from a type's capacity. It cannot see your bootstrap, so anything your +userData reserves has to be declared here as well: + +```yaml +spec: + kubelet: + systemReserved: {cpu: 200m, memory: 512Mi} + kubeReserved: {cpu: 200m, memory: 512Mi} + evictionHard: {memory.available: 400Mi} +``` + +**These values describe your userData; they do not configure it.** Setting them +here does not change what the node reserves, and the two must be kept in +agreement. Declaring less than the bootstrap actually reserves is the failure +worth knowing about: Karpenter then believes a server is larger than it is, +picks one too small for the pod, and the pod stays `Pending` while the empty node +is consolidated away and replaced — indefinitely, without an error anywhere. + +Karpenter's own `nodeclaim/consistency` check will not catch this. It compares +capacity rather than allocatable, and only reports a shortfall beyond 10%. + +Omitting the block leaves the previous behaviour in place — a flat `100m`/`100Mi` +reserve plus the kubelet's own default eviction thresholds — so upgrading cannot +silently raise a node's advertised capacity. Declaring a block replaces that +default with exactly what you state, so declare everything your bootstrap +reserves, not just the parts that differ. + +### Advertised vs. usable memory + +Hetzner advertises the memory a VM is allocated, but the guest kernel never sees +all of it — firmware, the kernel image and per-page structures take a cut that +grows with the size of the machine. A server advertised as 32Gi reports about +31.3Gi; one advertised as 4Gi reports about 3.7Gi. + +The provider holds back `VM_MEMORY_OVERHEAD_PERCENT` (default `0.075`) to cover +this. It is only an estimate, and it is only used until a node of that server +type and image registers: from then on the capacity that node reported is used +instead, keyed by server type and resolved image. Look for +`recorded server type memory capacity measured from a registered node` in the +logs. + +If nodes still register smaller than Karpenter expected, raise the percentage. +Lower it only with measurements in hand. + ## Examples Ready-to-apply examples live in the [`examples/`](examples/) directory. Each @@ -177,6 +227,7 @@ comments explaining every field. |---------|----------|-------------| | `HCLOUD_TOKEN` | yes | Hetzner Cloud API token | | `CLUSTER_NAME` | yes | Cluster identifier; scopes managed servers | +| `VM_MEMORY_OVERHEAD_PERCENT` | no (0.075) | Fraction of advertised memory assumed invisible to the guest, used until a node of that type reports its real capacity. Must be `>= 0` and `< 1`. | | `METRICS_PORT` | no (8080) | Prometheus metrics port | | `HEALTH_PROBE_PORT` | no (8081) | Health/readiness probe port | diff --git a/charts/karpenter-provider-hetzner/crds/karpenter.hetzner.cloud_hcloudnodeclasses.yaml b/charts/karpenter-provider-hetzner/crds/karpenter.hetzner.cloud_hcloudnodeclasses.yaml index 6cdb2e6..c4d64cd 100644 --- a/charts/karpenter-provider-hetzner/crds/karpenter.hetzner.cloud_hcloudnodeclasses.yaml +++ b/charts/karpenter-provider-hetzner/crds/karpenter.hetzner.cloud_hcloudnodeclasses.yaml @@ -83,6 +83,61 @@ spec: required: - family type: object + kubelet: + description: |- + Kubelet declares the reservations this node class's bootstrap applies to + the kubelet, so Karpenter can subtract them when computing how much of a + server a pod can actually use. + + This provider does not render the bootstrap: userData is supplied by the + operator, so these values describe what that userData does rather than + configure it. They must be kept in agreement with it. Declaring less than + the bootstrap reserves makes Karpenter believe a server is larger than it + is, which strands pods on nodes too small for them. + properties: + evictionHard: + additionalProperties: + type: string + description: |- + EvictionHard is the kubelet's hard eviction thresholds. Memory held back + for eviction is memory a pod cannot have, so it comes out of allocatable + the same way a reservation does. Values are either a quantity ("400Mi") or + a percentage of capacity ("10%"). + + Soft eviction is deliberately not modelled: it is a warning threshold that + does not reduce allocatable. + type: object + x-kubernetes-validations: + - message: valid keys for evictionHard are ['memory.available','nodefs.available','nodefs.inodesFree','imagefs.available','imagefs.inodesFree','pid.available'] + rule: self.all(x, x in ['memory.available','nodefs.available','nodefs.inodesFree','imagefs.available','imagefs.inodesFree','pid.available']) + kubeReserved: + additionalProperties: + type: string + description: |- + KubeReserved is resources reserved for Kubernetes system daemons, matching + the kubelet's --kube-reserved. + type: object + x-kubernetes-validations: + - message: valid keys for kubeReserved are ['cpu','memory','ephemeral-storage','pid'] + rule: self.all(x, x=='cpu' || x=='memory' || x=='ephemeral-storage' + || x=='pid') + - message: kubeReserved value cannot be a negative resource quantity + rule: self.all(x, !self[x].startsWith('-')) + systemReserved: + additionalProperties: + type: string + description: |- + SystemReserved is resources reserved for OS system daemons, matching the + kubelet's --system-reserved. + type: object + x-kubernetes-validations: + - message: valid keys for systemReserved are ['cpu','memory','ephemeral-storage','pid'] + rule: self.all(x, x=='cpu' || x=='memory' || x=='ephemeral-storage' + || x=='pid') + - message: systemReserved value cannot be a negative resource + quantity + rule: self.all(x, !self[x].startsWith('-')) + type: object labels: additionalProperties: type: string diff --git a/charts/karpenter-provider-hetzner/templates/deployment.yaml b/charts/karpenter-provider-hetzner/templates/deployment.yaml index 146d2a5..b63f241 100644 --- a/charts/karpenter-provider-hetzner/templates/deployment.yaml +++ b/charts/karpenter-provider-hetzner/templates/deployment.yaml @@ -68,6 +68,12 @@ spec: key: {{ .Values.auth.secretRef.key }} - name: CLUSTER_NAME value: {{ .Values.clusterName | quote }} + # Emitted unconditionally rather than through `with`, which skips + # falsy values: 0 is a legitimate setting (trust Hetzner's advertised + # memory exactly) and must reach the operator, not fall back to the + # default. + - name: VM_MEMORY_OVERHEAD_PERCENT + value: {{ .Values.vmMemoryOverheadPercent | quote }} - name: METRICS_PORT value: {{ .Values.metrics.port | quote }} - name: HEALTH_PROBE_PORT diff --git a/charts/karpenter-provider-hetzner/values.yaml b/charts/karpenter-provider-hetzner/values.yaml index 3e5cecf..9267fba 100644 --- a/charts/karpenter-provider-hetzner/values.yaml +++ b/charts/karpenter-provider-hetzner/values.yaml @@ -52,6 +52,16 @@ args: [] # Required: scopes managed servers so multiple clusters can share one Hetzner project. clusterName: "" +# Fraction of a server type's advertised memory assumed to be invisible to the +# guest. Hetzner advertises what the VM is allocated; the kernel and firmware +# take a cut that varies with machine size, so this is an estimate, used only +# until a node of that type registers and reports its real capacity. +# +# Raise it if nodes still register smaller than Karpenter expects. Lower it only +# with measurements in hand: too low tells Karpenter a server is bigger than it +# is, which strands pods on nodes that cannot hold them. +vmMemoryOverheadPercent: 0.075 + metrics: port: 8080 healthProbe: diff --git a/cmd/controller/main.go b/cmd/controller/main.go index e442fe8..e491737 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -15,6 +15,7 @@ import ( _ "github.com/paperclipinc/karpenter-provider-hetzner/pkg/apis/v1" hetznercp "github.com/paperclipinc/karpenter-provider-hetzner/pkg/cloudprovider" + instancetypecapacity "github.com/paperclipinc/karpenter-provider-hetzner/pkg/controllers/instancetype/capacity" "github.com/paperclipinc/karpenter-provider-hetzner/pkg/controllers/nodeclass" hetznerop "github.com/paperclipinc/karpenter-provider-hetzner/pkg/operator" "github.com/paperclipinc/karpenter-provider-hetzner/pkg/providers/imagefamily" @@ -40,7 +41,7 @@ func main() { // Create the three providers. instanceProvider := instance.NewProviderWithPlacementGroups(&hcloudClient.Server, &hcloudClient.PlacementGroup, cfg.ClusterName, &hcloudClient.Action) - typeProvider := instancetype.NewProvider(&hcloudClient.ServerType) + typeProvider := instancetype.NewProvider(&hcloudClient.ServerType, cfg.VMMemoryOverheadPercent) imageProvider := imagefamily.NewProvider(&hcloudClient.Image) // Create the cloud provider. @@ -74,5 +75,6 @@ func main() { op.InstanceTypeStore, ), nodeClassController, + instancetypecapacity.NewController(op.GetClient(), typeProvider), )...).Start(ctx) } diff --git a/pkg/apis/v1/hcloudnodeclass_types.go b/pkg/apis/v1/hcloudnodeclass_types.go index 6a68222..f5f92af 100644 --- a/pkg/apis/v1/hcloudnodeclass_types.go +++ b/pkg/apis/v1/hcloudnodeclass_types.go @@ -68,6 +68,48 @@ type HCloudNodeClassSpec struct { // +kubebuilder:default=true // +optional EnablePublicIPv6 *bool `json:"enablePublicIPv6,omitempty"` + + // Kubelet declares the reservations this node class's bootstrap applies to + // the kubelet, so Karpenter can subtract them when computing how much of a + // server a pod can actually use. + // + // This provider does not render the bootstrap: userData is supplied by the + // operator, so these values describe what that userData does rather than + // configure it. They must be kept in agreement with it. Declaring less than + // the bootstrap reserves makes Karpenter believe a server is larger than it + // is, which strands pods on nodes too small for them. + // +optional + Kubelet *KubeletConfiguration `json:"kubelet,omitempty"` +} + +// KubeletConfiguration mirrors the subset of kubelet settings that change how +// much of a node is schedulable. Only the reservation knobs are modelled; +// anything that does not move allocatable is deliberately absent. +type KubeletConfiguration struct { + // SystemReserved is resources reserved for OS system daemons, matching the + // kubelet's --system-reserved. + // +kubebuilder:validation:XValidation:message="valid keys for systemReserved are ['cpu','memory','ephemeral-storage','pid']",rule="self.all(x, x=='cpu' || x=='memory' || x=='ephemeral-storage' || x=='pid')" + // +kubebuilder:validation:XValidation:message="systemReserved value cannot be a negative resource quantity",rule="self.all(x, !self[x].startsWith('-'))" + // +optional + SystemReserved map[string]string `json:"systemReserved,omitempty"` + + // KubeReserved is resources reserved for Kubernetes system daemons, matching + // the kubelet's --kube-reserved. + // +kubebuilder:validation:XValidation:message="valid keys for kubeReserved are ['cpu','memory','ephemeral-storage','pid']",rule="self.all(x, x=='cpu' || x=='memory' || x=='ephemeral-storage' || x=='pid')" + // +kubebuilder:validation:XValidation:message="kubeReserved value cannot be a negative resource quantity",rule="self.all(x, !self[x].startsWith('-'))" + // +optional + KubeReserved map[string]string `json:"kubeReserved,omitempty"` + + // EvictionHard is the kubelet's hard eviction thresholds. Memory held back + // for eviction is memory a pod cannot have, so it comes out of allocatable + // the same way a reservation does. Values are either a quantity ("400Mi") or + // a percentage of capacity ("10%"). + // + // Soft eviction is deliberately not modelled: it is a warning threshold that + // does not reduce allocatable. + // +kubebuilder:validation:XValidation:message="valid keys for evictionHard are ['memory.available','nodefs.available','nodefs.inodesFree','imagefs.available','imagefs.inodesFree','pid.available']",rule="self.all(x, x in ['memory.available','nodefs.available','nodefs.inodesFree','imagefs.available','imagefs.inodesFree','pid.available'])" + // +optional + EvictionHard map[string]string `json:"evictionHard,omitempty"` } // UserDataSecretReference points at a Secret key holding the server userData. diff --git a/pkg/apis/v1/zz_generated.deepcopy.go b/pkg/apis/v1/zz_generated.deepcopy.go index 2a09032..ffefb43 100644 --- a/pkg/apis/v1/zz_generated.deepcopy.go +++ b/pkg/apis/v1/zz_generated.deepcopy.go @@ -109,6 +109,11 @@ func (in *HCloudNodeClassSpec) DeepCopyInto(out *HCloudNodeClassSpec) { *out = new(bool) **out = **in } + if in.Kubelet != nil { + in, out := &in.Kubelet, &out.Kubelet + *out = new(KubeletConfiguration) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HCloudNodeClassSpec. @@ -170,6 +175,42 @@ func (in *ImageSelector) DeepCopy() *ImageSelector { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *KubeletConfiguration) DeepCopyInto(out *KubeletConfiguration) { + *out = *in + if in.SystemReserved != nil { + in, out := &in.SystemReserved, &out.SystemReserved + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.KubeReserved != nil { + in, out := &in.KubeReserved, &out.KubeReserved + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.EvictionHard != nil { + in, out := &in.EvictionHard, &out.EvictionHard + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubeletConfiguration. +func (in *KubeletConfiguration) DeepCopy() *KubeletConfiguration { + if in == nil { + return nil + } + out := new(KubeletConfiguration) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResolvedImage) DeepCopyInto(out *ResolvedImage) { *out = *in diff --git a/pkg/cloudprovider/cloudprovider.go b/pkg/cloudprovider/cloudprovider.go index c23cd67..419a61d 100644 --- a/pkg/cloudprovider/cloudprovider.go +++ b/pkg/cloudprovider/cloudprovider.go @@ -94,7 +94,7 @@ func (cp *CloudProvider) Create(ctx context.Context, nodeClaim *karpv1.NodeClaim } // Get instance types for the node class locations. - instanceTypes, err := cp.typeProvider.List(ctx, nodeClass.Spec.Locations) + instanceTypes, err := cp.typeProvider.List(ctx, nodeClass) if err != nil { return nil, fmt.Errorf("listing instance types: %w", err) } @@ -251,7 +251,7 @@ func (cp *CloudProvider) GetInstanceTypes(ctx context.Context, nodePool *karpv1. return nil, fmt.Errorf("resolving node class for node pool %s: %w", nodePool.Name, err) } - return cp.typeProvider.List(ctx, nodeClass.Spec.Locations) + return cp.typeProvider.List(ctx, nodeClass) } // IsDrifted determines whether the given NodeClaim has drifted from its desired state. diff --git a/pkg/cloudprovider/cloudprovider_test.go b/pkg/cloudprovider/cloudprovider_test.go index 0f94365..bcf6635 100644 --- a/pkg/cloudprovider/cloudprovider_test.go +++ b/pkg/cloudprovider/cloudprovider_test.go @@ -134,7 +134,7 @@ func buildCP(t *testing.T, nc *apiv1.HCloudNodeClass, server *hcloud.Server) (*c cp := cloudprovider.NewCloudProvider(kube, instance.NewProvider(fsc, "test-cluster"), - instancetype.NewProvider(stc), + instancetype.NewProvider(stc, 0), imagefamily.NewProvider(imgc)) nodeClaim := &karpv1.NodeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim"}} @@ -207,7 +207,7 @@ func buildCPWithTypes(t *testing.T, nc *apiv1.HCloudNodeClass, types []*hcloud.S fsc := &fakeServerClient{servers: map[int64]*hcloud.Server{}} stc := &fakeServerTypeClient{types: types} imgc := &fakeImageClient{images: []*hcloud.Image{{ID: 42, Description: "Ubuntu 24.04", Architecture: hcloud.ArchitectureX86}}} - typeProvider := instancetype.NewProvider(stc) + typeProvider := instancetype.NewProvider(stc, 0) cp := cloudprovider.NewCloudProvider(kube, instance.NewProvider(fsc, "test-cluster"), typeProvider, @@ -308,7 +308,7 @@ func TestCreate_InsufficientCapacityMarksUnavailable(t *testing.T) { t.Fatal("expected error on capacity failure") } // The offering for (cx22, nbg1) should now be marked unavailable. - its, lerr := typeProvider.List(context.Background(), []string{"nbg1"}) + its, lerr := typeProvider.List(context.Background(), &apiv1.HCloudNodeClass{Spec: apiv1.HCloudNodeClassSpec{Locations: []string{"nbg1"}}}) if lerr != nil { t.Fatal(lerr) } @@ -460,7 +460,7 @@ func TestCreate_UserDataFromSecret(t *testing.T) { imgc := &fakeImageClient{images: []*hcloud.Image{{ID: 42, Description: "Ubuntu 24.04", Architecture: hcloud.ArchitectureX86}}} cp := cloudprovider.NewCloudProvider(kube, instance.NewProvider(fsc, "test-cluster"), - instancetype.NewProvider(stc), + instancetype.NewProvider(stc, 0), imagefamily.NewProvider(imgc)) if _, err := cp.Create(context.Background(), createNodeClaim()); err != nil { @@ -485,7 +485,7 @@ func TestCreate_UserDataInlineWhenNoRef(t *testing.T) { imgc := &fakeImageClient{images: []*hcloud.Image{{ID: 42, Description: "Ubuntu 24.04", Architecture: hcloud.ArchitectureX86}}} cp := cloudprovider.NewCloudProvider(kube, instance.NewProvider(fsc, "test-cluster"), - instancetype.NewProvider(stc), + instancetype.NewProvider(stc, 0), imagefamily.NewProvider(imgc)) if _, err := cp.Create(context.Background(), createNodeClaim()); err != nil { @@ -519,7 +519,7 @@ func TestCreate_UserDataSecretKeyMissing(t *testing.T) { imgc := &fakeImageClient{images: []*hcloud.Image{{ID: 42, Description: "Ubuntu 24.04", Architecture: hcloud.ArchitectureX86}}} cp := cloudprovider.NewCloudProvider(kube, instance.NewProvider(fsc, "test-cluster"), - instancetype.NewProvider(stc), + instancetype.NewProvider(stc, 0), imagefamily.NewProvider(imgc)) _, err := cp.Create(context.Background(), createNodeClaim()) @@ -546,7 +546,7 @@ func TestCreate_UserDataSecretMissing(t *testing.T) { imgc := &fakeImageClient{images: []*hcloud.Image{{ID: 42, Description: "Ubuntu 24.04", Architecture: hcloud.ArchitectureX86}}} cp := cloudprovider.NewCloudProvider(kube, instance.NewProvider(fsc, "test-cluster"), - instancetype.NewProvider(stc), + instancetype.NewProvider(stc, 0), imagefamily.NewProvider(imgc)) _, err := cp.Create(context.Background(), createNodeClaim()) diff --git a/pkg/controllers/instancetype/capacity/controller.go b/pkg/controllers/instancetype/capacity/controller.go new file mode 100644 index 0000000..9a242c6 --- /dev/null +++ b/pkg/controllers/instancetype/capacity/controller.go @@ -0,0 +1,170 @@ +// Package capacity records the memory capacity Hetzner servers actually report +// once they boot, so Karpenter can size nodes from measurements instead of an +// estimate. +// +// Hetzner advertises the memory a VM is allocated, not what the guest kernel +// ends up seeing. The difference is a few percent, it varies with the size of +// the machine, and it is invisible from the API -- so the instance type provider +// falls back to a single conservative fraction until a real node of that type +// reports its own figure. This controller supplies that figure. +// +// The AWS provider solves the same problem the same way, for the same reason. +package capacity + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1" + + apiv1 "github.com/paperclipinc/karpenter-provider-hetzner/pkg/apis/v1" +) + +// CapacityRecorder is the instance type provider's seam for accepting a +// measurement. It reports whether the observation changed what the provider will +// use, so that only first discoveries and genuine drops are logged. +type CapacityRecorder interface { + Record(serverType string, nodeClass *apiv1.HCloudNodeClass, observed resource.Quantity) bool +} + +// Controller records capacity from registered nodes. +type Controller struct { + kubeClient client.Client + recorder CapacityRecorder +} + +func NewController(kubeClient client.Client, recorder CapacityRecorder) *Controller { + return &Controller{kubeClient: kubeClient, recorder: recorder} +} + +func (c *Controller) Name() string { + return "instancetype.capacity" +} + +// Reconcile reads one registered node's reported memory capacity and hands it to +// the instance type provider. +// +// Every path that cannot produce a trustworthy measurement returns without +// recording rather than returning an error. A node whose node pool or node class +// has been deleted is a race with teardown, not a fault, and requeueing it would +// spin against objects that are never coming back. +func (c *Controller) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { + node := &corev1.Node{} + if err := c.kubeClient.Get(ctx, req.NamespacedName, node); err != nil { + if apierrors.IsNotFound(err) { + return reconcile.Result{}, nil + } + return reconcile.Result{}, fmt.Errorf("getting node %s: %w", req.Name, err) + } + + // Only a registered node has finished joining and settled on the capacity it + // will report for the rest of its life. + if node.Labels[karpv1.NodeRegisteredLabelKey] != "true" { + return reconcile.Result{}, nil + } + + serverType := node.Labels[corev1.LabelInstanceTypeStable] + if serverType == "" { + return reconcile.Result{}, nil + } + + observed := node.Status.Capacity[corev1.ResourceMemory] + // Zero is the absence of a reading, not a very small machine. Recording it + // would make Karpenter believe the type holds nothing and strand every pod + // scheduled onto it. + if observed.Sign() <= 0 { + return reconcile.Result{}, nil + } + + nodeClass, err := c.nodeClassFor(ctx, node) + if err != nil || nodeClass == nil { + return reconcile.Result{}, err + } + + if c.recorder.Record(serverType, nodeClass, observed) { + log.FromContext(ctx).WithValues( + "serverType", serverType, + "nodeClass", nodeClass.Name, + "node", node.Name, + "capacity", observed.String(), + ).Info("recorded server type memory capacity measured from a registered node") + } + return reconcile.Result{}, nil +} + +// nodeClassFor resolves the node's node pool and, through it, the node class the +// server was built from. It returns nil without error when the node is not ours +// or when either object has already been deleted. +func (c *Controller) nodeClassFor(ctx context.Context, node *corev1.Node) (*apiv1.HCloudNodeClass, error) { + poolName := node.Labels[karpv1.NodePoolLabelKey] + if poolName == "" { + return nil, nil + } + + nodePool := &karpv1.NodePool{} + if err := c.kubeClient.Get(ctx, client.ObjectKey{Name: poolName}, nodePool); err != nil { + if apierrors.IsNotFound(err) { + return nil, nil + } + return nil, fmt.Errorf("getting nodepool %s: %w", poolName, err) + } + + ref := nodePool.Spec.Template.Spec.NodeClassRef + // A node pool belonging to another provider says nothing about how Hetzner + // sizes a server, even if the label happens to name a type we know. + if ref == nil || ref.Group != apiv1.Group || ref.Kind != "HCloudNodeClass" { + return nil, nil + } + + nodeClass := &apiv1.HCloudNodeClass{} + if err := c.kubeClient.Get(ctx, client.ObjectKey{Name: ref.Name}, nodeClass); err != nil { + if apierrors.IsNotFound(err) { + return nil, nil + } + return nil, fmt.Errorf("getting hcloudnodeclass %s: %w", ref.Name, err) + } + return nodeClass, nil +} + +// Register wires the controller to nodes becoming registered. +// +// A node's reported capacity does not change after it joins, so there is nothing +// to learn from watching it afterwards. The predicates narrow the watch to the +// transition into registration, plus already-registered nodes at startup so a +// fresh process rebuilds its cache from the running fleet rather than waiting for +// the next launch. +func (c *Controller) Register(_ context.Context, m manager.Manager) error { + return controllerruntime.NewControllerManagedBy(m). + For(&corev1.Node{}, builder.WithPredicates(predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return e.Object.GetLabels()[karpv1.NodeRegisteredLabelKey] == "true" + }, + UpdateFunc: func(e event.UpdateEvent) bool { + // Only the moment registration is gained; a node that was already + // registered has nothing new to say. + if e.ObjectOld.GetLabels()[karpv1.NodeRegisteredLabelKey] != "" { + return false + } + return e.ObjectNew.GetLabels()[karpv1.NodeRegisteredLabelKey] == "true" + }, + DeleteFunc: func(event.DeleteEvent) bool { return false }, + GenericFunc: func(event.GenericEvent) bool { return false }, + })). + Named(c.Name()). + // One worker: the work is a map write behind a mutex, and serialising it + // keeps contention off the provider's read path. + WithOptions(controller.Options{MaxConcurrentReconciles: 1}). + Complete(c) +} diff --git a/pkg/controllers/instancetype/capacity/controller_test.go b/pkg/controllers/instancetype/capacity/controller_test.go new file mode 100644 index 0000000..50f5049 --- /dev/null +++ b/pkg/controllers/instancetype/capacity/controller_test.go @@ -0,0 +1,195 @@ +package capacity + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1" + + apiv1 "github.com/paperclipinc/karpenter-provider-hetzner/pkg/apis/v1" +) + +// recordedCall captures what the controller handed the provider. +type recordedCall struct { + serverType string + nodeClass string + observed resource.Quantity +} + +type fakeRecorder struct{ calls []recordedCall } + +func (f *fakeRecorder) Record(serverType string, nodeClass *apiv1.HCloudNodeClass, observed resource.Quantity) bool { + name := "" + if nodeClass != nil { + name = nodeClass.Name + } + f.calls = append(f.calls, recordedCall{serverType: serverType, nodeClass: name, observed: observed}) + return true +} + +func testNodeClass() *apiv1.HCloudNodeClass { + return &apiv1.HCloudNodeClass{ + ObjectMeta: metav1.ObjectMeta{Name: "default"}, + Status: apiv1.HCloudNodeClassStatus{ + ResolvedImages: []apiv1.ResolvedImage{{Architecture: "x86", ImageID: 42}}, + }, + } +} + +func testNodePool(group, kind string) *karpv1.NodePool { + return &karpv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{Name: "workers"}, + Spec: karpv1.NodePoolSpec{ + Template: karpv1.NodeClaimTemplate{ + Spec: karpv1.NodeClaimTemplateSpec{ + NodeClassRef: &karpv1.NodeClassReference{Name: "default", Group: group, Kind: kind}, + }, + }, + }, + } +} + +// testNode is a registered Karpenter-owned node reporting a real cx53's capacity. +func testNode() *corev1.Node { + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "worker-1", + Labels: map[string]string{ + corev1.LabelInstanceTypeStable: "cx53", + karpv1.NodePoolLabelKey: "workers", + karpv1.NodeRegisteredLabelKey: "true", + }, + }, + Status: corev1.NodeStatus{ + Capacity: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("32089152Ki"), + }, + }, + } +} + +func reconcileNode(t *testing.T, node *corev1.Node, objs ...client.Object) *fakeRecorder { + t.Helper() + _ = apiv1.SchemeBuilder.AddToScheme(scheme.Scheme) + + all := append([]client.Object{node}, objs...) + kube := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(all...).Build() + + rec := &fakeRecorder{} + c := NewController(kube, rec) + if _, err := c.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: node.Name}, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + return rec +} + +func TestReconcile_RecordsRegisteredNodeCapacity(t *testing.T) { + rec := reconcileNode(t, testNode(), testNodePool(apiv1.Group, "HCloudNodeClass"), testNodeClass()) + + if len(rec.calls) != 1 { + t.Fatalf("expected 1 recorded observation, got %d", len(rec.calls)) + } + got := rec.calls[0] + if got.serverType != "cx53" { + t.Errorf("expected server type cx53, got %q", got.serverType) + } + if got.nodeClass != "default" { + t.Errorf("expected node class default, got %q", got.nodeClass) + } + if got.observed.String() != "32089152Ki" { + t.Errorf("expected the node's reported capacity, got %s", got.observed.String()) + } +} + +// An unregistered node has not finished joining, so its reported capacity is not +// yet trustworthy. +func TestReconcile_IgnoresUnregisteredNode(t *testing.T) { + node := testNode() + delete(node.Labels, karpv1.NodeRegisteredLabelKey) + + rec := reconcileNode(t, node, testNodePool(apiv1.Group, "HCloudNodeClass"), testNodeClass()) + if len(rec.calls) != 0 { + t.Errorf("expected no observation from an unregistered node, got %d", len(rec.calls)) + } +} + +// Nodes Karpenter does not own tell us nothing about a Hetzner server type as +// this provider builds it. +func TestReconcile_IgnoresNodeWithoutNodePool(t *testing.T) { + node := testNode() + delete(node.Labels, karpv1.NodePoolLabelKey) + + rec := reconcileNode(t, node) + if len(rec.calls) != 0 { + t.Errorf("expected no observation from a node with no node pool, got %d", len(rec.calls)) + } +} + +// A node pool pointing at another provider's node class is not ours to measure. +// +// The HCloudNodeClass is seeded deliberately: without it the lookup would 404 +// and the test would pass whether or not the group and kind are checked at all. +func TestReconcile_IgnoresForeignNodeClass(t *testing.T) { + rec := reconcileNode(t, testNode(), testNodePool("karpenter.k8s.aws", "EC2NodeClass"), testNodeClass()) + if len(rec.calls) != 0 { + t.Errorf("expected no observation for a foreign node class, got %d", len(rec.calls)) + } +} + +func TestReconcile_IgnoresNodeWithoutInstanceType(t *testing.T) { + node := testNode() + delete(node.Labels, corev1.LabelInstanceTypeStable) + + rec := reconcileNode(t, node, testNodePool(apiv1.Group, "HCloudNodeClass"), testNodeClass()) + if len(rec.calls) != 0 { + t.Errorf("expected no observation without an instance type, got %d", len(rec.calls)) + } +} + +// A node reporting no memory is reporting an absence of data, not a very small +// machine; recording it would strand every pod on that type. +func TestReconcile_IgnoresZeroCapacity(t *testing.T) { + node := testNode() + node.Status.Capacity = corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("0")} + + rec := reconcileNode(t, node, testNodePool(apiv1.Group, "HCloudNodeClass"), testNodeClass()) + if len(rec.calls) != 0 { + t.Errorf("expected no observation for zero capacity, got %d", len(rec.calls)) + } +} + +// A missing node pool or node class is a race with deletion, not an error worth +// retrying: the node is on its way out. +func TestReconcile_ToleratesMissingNodePool(t *testing.T) { + rec := reconcileNode(t, testNode()) + if len(rec.calls) != 0 { + t.Errorf("expected no observation when the node pool is gone, got %d", len(rec.calls)) + } +} + +// A deleted node must not error the reconcile loop. +func TestReconcile_ToleratesMissingNode(t *testing.T) { + _ = apiv1.SchemeBuilder.AddToScheme(scheme.Scheme) + kube := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + rec := &fakeRecorder{} + c := NewController(kube, rec) + + if _, err := c.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: "gone"}, + }); err != nil { + t.Fatalf("expected no error for a missing node, got %v", err) + } + if len(rec.calls) != 0 { + t.Errorf("expected no observation, got %d", len(rec.calls)) + } +} diff --git a/pkg/operator/config.go b/pkg/operator/config.go index 3590fdf..87a215c 100644 --- a/pkg/operator/config.go +++ b/pkg/operator/config.go @@ -4,16 +4,37 @@ import ( "fmt" "os" "regexp" + "strconv" "strings" ) var clusterNameRE = regexp.MustCompile(`^[a-zA-Z0-9._-]{1,63}$`) +// DefaultVMMemoryOverheadPercent is the fraction of a server type's advertised +// memory assumed to be unavailable to the guest. +// +// Hetzner advertises the RAM the VM is allocated, but the guest kernel never +// sees all of it: firmware, the kernel image and per-page structures take a cut +// that grows with the size of the machine. Measured across cx/cpx types the gap +// runs from roughly 4.4% on a 32Gi server to 6.9% on a 4Gi one, so a single +// fraction cannot be exact everywhere. 0.075 is deliberately on the safe side of +// every measurement: erring high costs a little schedulable memory, while erring +// low tells Karpenter a server is bigger than it is and strands pods on it. +// +// This is only the estimate used before a node of that type has been seen. Once +// one registers, its real capacity is recorded and used instead, so the constant +// matters for the first node of each server type and image, not the fleet. +const DefaultVMMemoryOverheadPercent = 0.075 + // Config holds provider configuration sourced from the environment. type Config struct { // ClusterName scopes all managed servers so multiple clusters can share // one Hetzner project without colliding. ClusterName string + + // VMMemoryOverheadPercent is subtracted from every server type's advertised + // memory. See DefaultVMMemoryOverheadPercent. + VMMemoryOverheadPercent float64 } // LoadConfig reads provider configuration from the environment. @@ -26,5 +47,30 @@ func LoadConfig() (*Config, error) { if !clusterNameRE.MatchString(name) { return nil, fmt.Errorf("CLUSTER_NAME %q is not a valid Hetzner label value (must match [a-zA-Z0-9._-], max 63 chars)", name) } - return &Config{ClusterName: name}, nil + + overhead, err := parseVMMemoryOverheadPercent(os.Getenv("VM_MEMORY_OVERHEAD_PERCENT")) + if err != nil { + return nil, err + } + + return &Config{ClusterName: name, VMMemoryOverheadPercent: overhead}, nil +} + +// parseVMMemoryOverheadPercent reads the overhead fraction, defaulting when +// unset. A value of 1 or more would leave a server with no memory at all, and a +// negative one would claim more memory than Hetzner sells; both are rejected at +// startup rather than silently producing unschedulable or oversized nodes. +func parseVMMemoryOverheadPercent(raw string) (float64, error) { + raw = strings.TrimSpace(raw) + if raw == "" { + return DefaultVMMemoryOverheadPercent, nil + } + v, err := strconv.ParseFloat(raw, 64) + if err != nil { + return 0, fmt.Errorf("VM_MEMORY_OVERHEAD_PERCENT %q is not a number (expected a fraction such as 0.075)", raw) + } + if v < 0 || v >= 1 { + return 0, fmt.Errorf("VM_MEMORY_OVERHEAD_PERCENT %v is out of range (must be at least 0 and less than 1)", v) + } + return v, nil } diff --git a/pkg/operator/config_test.go b/pkg/operator/config_test.go index 71a82c1..bdcbadf 100644 --- a/pkg/operator/config_test.go +++ b/pkg/operator/config_test.go @@ -28,3 +28,41 @@ func TestLoadConfig_RejectsInvalidClusterName(t *testing.T) { } } } + +func TestLoadConfig_VMMemoryOverheadPercent(t *testing.T) { + tests := []struct { + name string + env string + want float64 + wantErr bool + }{ + {name: "unset uses default", env: "", want: 0.075}, + {name: "explicit value", env: "0.1", want: 0.1}, + {name: "zero is allowed", env: "0", want: 0}, + {name: "negative rejected", env: "-0.1", wantErr: true}, + {name: "one rejected", env: "1", wantErr: true}, + {name: "above one rejected", env: "1.5", wantErr: true}, + {name: "non-numeric rejected", env: "7.5%", wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("CLUSTER_NAME", "test-cluster") + if tc.env != "" { + t.Setenv("VM_MEMORY_OVERHEAD_PERCENT", tc.env) + } + cfg, err := LoadConfig() + if tc.wantErr { + if err == nil { + t.Fatalf("expected error for %q, got config %+v", tc.env, cfg) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.VMMemoryOverheadPercent != tc.want { + t.Errorf("expected %v, got %v", tc.want, cfg.VMMemoryOverheadPercent) + } + }) + } +} diff --git a/pkg/providers/instancetype/allocatable.go b/pkg/providers/instancetype/allocatable.go new file mode 100644 index 0000000..ff48a56 --- /dev/null +++ b/pkg/providers/instancetype/allocatable.go @@ -0,0 +1,144 @@ +package instancetype + +import ( + "strconv" + "strings" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "sigs.k8s.io/karpenter/pkg/cloudprovider" + + apiv1 "github.com/paperclipinc/karpenter-provider-hetzner/pkg/apis/v1" +) + +// Kubelet's own defaults, applied when the node class declares no eviction +// thresholds. The kubelet always holds something back even when nothing is +// configured, so modelling zero would overstate what a pod can use. +const ( + defaultMemoryEvictionThreshold = "100Mi" + defaultNodefsEvictionThreshold = "10%" +) + +// overheadFor converts a node class's declared kubelet reservations into the +// overhead Karpenter subtracts from capacity. Karpenter's own formula is +// allocatable = capacity - (kubeReserved + systemReserved + evictionThreshold), +// so every one of those three has to be filled in for allocatable to match what +// the node will actually report. +// +// capacity is required because eviction thresholds may be expressed as a +// percentage of it. +func overheadFor(nodeClass *apiv1.HCloudNodeClass, capacity corev1.ResourceList) *cloudprovider.InstanceTypeOverhead { + var kubelet *apiv1.KubeletConfiguration + if nodeClass != nil { + kubelet = nodeClass.Spec.Kubelet + } + + overhead := &cloudprovider.InstanceTypeOverhead{ + EvictionThreshold: evictionThreshold(kubelet, capacity), + } + if kubelet == nil { + // A node class that declares nothing has said nothing about its bootstrap, + // which is not the same as saying it reserves nothing. Before this package + // read reservations from the node class it subtracted a flat 100m/100Mi + // from every type; keeping that as the undeclared default means upgrading + // cannot silently raise a node's advertised capacity, which would push pods + // onto machines that never had room for them. + overhead.KubeReserved = legacyDefaultKubeReserved() + return overhead + } + overhead.KubeReserved = parseResourceList(kubelet.KubeReserved) + overhead.SystemReserved = parseResourceList(kubelet.SystemReserved) + return overhead +} + +// legacyDefaultKubeReserved is what this provider reserved on every server type +// before reservations became declarable. It applies only when a node class omits +// the kubelet block entirely; one that declares a block is taken at its word. +func legacyDefaultKubeReserved() corev1.ResourceList { + return corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("100Mi"), + } +} + +// evictionThreshold models the memory and disk the kubelet holds back to keep +// itself above its hard eviction signals. That headroom is unavailable to pods, +// so it reduces allocatable exactly as a reservation does. +// +// Only signals that move a resource Karpenter schedules on are translated: +// memory.available and nodefs.available. inodesFree and pid.available are +// accepted by the API but have no allocatable equivalent. +func evictionThreshold(kubelet *apiv1.KubeletConfiguration, capacity corev1.ResourceList) corev1.ResourceList { + var hard map[string]string + if kubelet != nil { + hard = kubelet.EvictionHard + } + + signal := func(name, fallback string) string { + if v, ok := hard[name]; ok && strings.TrimSpace(v) != "" { + return v + } + return fallback + } + + threshold := corev1.ResourceList{} + if q, ok := resolveThreshold(signal("memory.available", defaultMemoryEvictionThreshold), capacity[corev1.ResourceMemory]); ok { + threshold[corev1.ResourceMemory] = q + } + if q, ok := resolveThreshold(signal("nodefs.available", defaultNodefsEvictionThreshold), capacity[corev1.ResourceEphemeralStorage]); ok { + threshold[corev1.ResourceEphemeralStorage] = q + } + return threshold +} + +// resolveThreshold reads an eviction threshold, which the kubelet accepts either +// as a quantity ("400Mi") or as a percentage of the resource's capacity ("10%"). +// An unparseable value yields no threshold rather than a zero one, so a typo +// cannot quietly hand pods memory the kubelet is holding back. +func resolveThreshold(raw string, capacity resource.Quantity) (resource.Quantity, bool) { + raw = strings.TrimSpace(raw) + if pct, ok := strings.CutSuffix(raw, "%"); ok { + f, err := strconv.ParseFloat(strings.TrimSpace(pct), 64) + if err != nil || f < 0 || f > 100 { + return resource.Quantity{}, false + } + return *resource.NewQuantity(int64(float64(capacity.Value())*f/100), resource.BinarySI), true + } + q, err := resource.ParseQuantity(raw) + if err != nil || q.Sign() < 0 { + return resource.Quantity{}, false + } + return q, true +} + +// parseResourceList converts the node class's string-keyed reservations into a +// ResourceList. Values that do not parse are dropped: the CEL rules on the CRD +// reject them at admission, so anything reaching here came in some other way and +// is safer ignored than treated as zero. +func parseResourceList(in map[string]string) corev1.ResourceList { + if len(in) == 0 { + return nil + } + out := corev1.ResourceList{} + for k, v := range in { + q, err := resource.ParseQuantity(strings.TrimSpace(v)) + if err != nil || q.Sign() < 0 { + continue + } + out[corev1.ResourceName(k)] = q + } + if len(out) == 0 { + return nil + } + return out +} + +// memoryWithVMOverhead reduces a server type's advertised memory to what the +// guest is expected to actually see. See operator.DefaultVMMemoryOverheadPercent +// for why the gap exists and why it is a fraction rather than a constant. +func memoryWithVMOverhead(advertisedBytes int64, overheadPercent float64) int64 { + if overheadPercent <= 0 { + return advertisedBytes + } + return int64(float64(advertisedBytes) * (1 - overheadPercent)) +} diff --git a/pkg/providers/instancetype/allocatable_test.go b/pkg/providers/instancetype/allocatable_test.go new file mode 100644 index 0000000..7124962 --- /dev/null +++ b/pkg/providers/instancetype/allocatable_test.go @@ -0,0 +1,190 @@ +package instancetype + +import ( + "context" + "testing" + + "github.com/hetznercloud/hcloud-go/v2/hcloud" + corev1 "k8s.io/api/core/v1" + + apiv1 "github.com/paperclipinc/karpenter-provider-hetzner/pkg/apis/v1" + "github.com/paperclipinc/karpenter-provider-hetzner/pkg/operator" +) + +// measuredNodes records what real Hetzner servers actually report once booted +// and registered, captured from running clusters. Hetzner's advertised size is +// not what the guest sees: the kernel and firmware reserve a slice that grows +// with RAM, so a cx53 advertised as 32Gi registers 31337Mi of capacity. On top +// of that the kubelet subtracts whatever the bootstrap reserved. +// +// These numbers are the reason this package cannot compute allocatable from the +// advertised size alone, and the reason the test below asserts an inequality +// rather than an exact figure: over-reporting allocatable makes Karpenter pick a +// node the pod cannot fit on, which strands the pod and churns the node forever. +// Under-reporting only costs money. +var measuredNodes = []struct { + name string + cores int + memGB float32 + realCapacityKi int64 + realAllocatableKi int64 +}{ + {"cpx22", 2, 4, 3905948, 2447772}, + {"cpx32", 4, 8, 7931612, 6473436}, + {"cx33", 4, 8, 7937224, 6479048}, + {"cx43", 8, 16, 15988560, 14530384}, + {"cx53", 16, 32, 32089152, 30630976}, +} + +// locationsNodeClass is a node class that only constrains locations, for tests +// about filtering rather than allocatable. +func locationsNodeClass(locations ...string) *apiv1.HCloudNodeClass { + return &apiv1.HCloudNodeClass{Spec: apiv1.HCloudNodeClassSpec{Locations: locations}} +} + +// k3sNodeClass mirrors the kubelet reservations these clusters' bootstrap sets: +// system-reserved and kube-reserved at 512Mi/200m each, plus a 400Mi hard +// eviction threshold. 1424Mi and 400m in total, on every server type. +func k3sNodeClass() *apiv1.HCloudNodeClass { + return &apiv1.HCloudNodeClass{ + Spec: apiv1.HCloudNodeClassSpec{ + Kubelet: &apiv1.KubeletConfiguration{ + SystemReserved: map[string]string{"cpu": "200m", "memory": "512Mi"}, + KubeReserved: map[string]string{"cpu": "200m", "memory": "512Mi"}, + EvictionHard: map[string]string{"memory.available": "400Mi"}, + }, + }, + } +} + +// TestAllocatable_NeverExceedsRegisteredNode is the regression for the churn +// loop: Karpenter sized a node from an allocatable figure larger than the +// machine's real capacity, the pod stayed Pending, the empty node was reclaimed, +// and provisioning repeated indefinitely. +func TestAllocatable_NeverExceedsRegisteredNode(t *testing.T) { + for _, tc := range measuredNodes { + t.Run(tc.name, func(t *testing.T) { + st := makeServerType(tc.name, hcloud.ArchitectureX86, hcloud.CPUTypeShared, tc.cores, tc.memGB, 80, testPricings) + client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} + p := NewProvider(client, operator.DefaultVMMemoryOverheadPercent) + + types, err := p.List(context.Background(), k3sNodeClass()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(types) != 1 { + t.Fatalf("expected 1 instance type, got %d", len(types)) + } + + alloc := types[0].Allocatable() + gotMem := alloc[corev1.ResourceMemory] + realMem := tc.realAllocatableKi * 1024 + if gotMem.Value() > realMem { + t.Errorf("memory allocatable %dMi exceeds what the node registers (%dMi): overestimates by %dMi", + gotMem.Value()/1024/1024, realMem/1024/1024, (gotMem.Value()-realMem)/1024/1024) + } + + gotCPU := alloc[corev1.ResourceCPU] + realCPU := int64(tc.cores)*1000 - 400 + if gotCPU.MilliValue() > realCPU { + t.Errorf("cpu allocatable %dm exceeds what the node registers (%dm)", gotCPU.MilliValue(), realCPU) + } + }) + } +} + +// TestAllocatable_WithinBudgetOfRegisteredNode guards the other direction. The +// estimate must be conservative, but an estimate far below reality silently +// buys larger servers than the workload needs, so hold it to a bounded shortfall. +func TestAllocatable_WithinBudgetOfRegisteredNode(t *testing.T) { + const maxShortfallMi = 1200 + + for _, tc := range measuredNodes { + t.Run(tc.name, func(t *testing.T) { + st := makeServerType(tc.name, hcloud.ArchitectureX86, hcloud.CPUTypeShared, tc.cores, tc.memGB, 80, testPricings) + client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} + p := NewProvider(client, operator.DefaultVMMemoryOverheadPercent) + + types, err := p.List(context.Background(), k3sNodeClass()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + gotMem := types[0].Allocatable()[corev1.ResourceMemory] + shortfallMi := (tc.realAllocatableKi*1024 - gotMem.Value()) / 1024 / 1024 + if shortfallMi > maxShortfallMi { + t.Errorf("memory allocatable %dMi understates the node by %dMi (budget %dMi)", + gotMem.Value()/1024/1024, shortfallMi, maxShortfallMi) + } + }) + } +} + +// TestAllocatable_NoKubeletConfigKeepsLegacyReservation pins the upgrade path. +// +// Before this package read reservations from the node class it subtracted a flat +// 100m/100Mi from every type. A node class that declares no kubelet block has +// said nothing about its bootstrap, so silently dropping that to zero would raise +// advertised CPU on upgrade -- the wrong direction, and invisible. Absent means +// "unchanged", not "nothing reserved". +func TestAllocatable_NoKubeletConfigKeepsLegacyReservation(t *testing.T) { + st := makeServerType("cx23", hcloud.ArchitectureX86, hcloud.CPUTypeShared, 2, 4, 40, testPricings) + client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} + p := NewProvider(client, operator.DefaultVMMemoryOverheadPercent) + + types, err := p.List(context.Background(), &apiv1.HCloudNodeClass{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + cpu := types[0].Allocatable()[corev1.ResourceCPU] + if cpu.MilliValue() != 1900 { + t.Errorf("expected 1900m (2 cores less the legacy 100m reserve), got %dm", cpu.MilliValue()) + } +} + +// A node class that does declare a kubelet block is taken at its word: the +// legacy default must not be added on top of what the operator stated. +func TestAllocatable_DeclaredKubeletOverridesLegacyDefault(t *testing.T) { + st := makeServerType("cx23", hcloud.ArchitectureX86, hcloud.CPUTypeShared, 2, 4, 40, testPricings) + client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} + p := NewProvider(client, operator.DefaultVMMemoryOverheadPercent) + + nc := &apiv1.HCloudNodeClass{ + Spec: apiv1.HCloudNodeClassSpec{ + Kubelet: &apiv1.KubeletConfiguration{ + KubeReserved: map[string]string{"cpu": "250m"}, + }, + }, + } + types, err := p.List(context.Background(), nc) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + cpu := types[0].Allocatable()[corev1.ResourceCPU] + if cpu.MilliValue() != 1750 { + t.Errorf("expected 1750m (2 cores less the declared 250m), got %dm", cpu.MilliValue()) + } +} + +// TestAllocatable_NoKubeletConfig falls back to advertised-minus-VM-overhead +// when the NodeClass declares no reservations. The result must still not exceed +// the machine's real capacity, since the VM overhead applies regardless. +func TestAllocatable_NoKubeletConfig(t *testing.T) { + tc := measuredNodes[4] // cx53 + st := makeServerType(tc.name, hcloud.ArchitectureX86, hcloud.CPUTypeShared, tc.cores, tc.memGB, 80, testPricings) + client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} + p := NewProvider(client, operator.DefaultVMMemoryOverheadPercent) + + types, err := p.List(context.Background(), &apiv1.HCloudNodeClass{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + gotMem := types[0].Allocatable()[corev1.ResourceMemory] + if gotMem.Value() > tc.realCapacityKi*1024 { + t.Errorf("memory allocatable %dMi exceeds real capacity %dMi with no kubelet config", + gotMem.Value()/1024/1024, tc.realCapacityKi/1024) + } +} diff --git a/pkg/providers/instancetype/discovered.go b/pkg/providers/instancetype/discovered.go new file mode 100644 index 0000000..b14795f --- /dev/null +++ b/pkg/providers/instancetype/discovered.go @@ -0,0 +1,96 @@ +package instancetype + +import ( + "fmt" + "sort" + "strings" + "sync" + + "k8s.io/apimachinery/pkg/api/resource" + + apiv1 "github.com/paperclipinc/karpenter-provider-hetzner/pkg/apis/v1" +) + +// discoveredCapacityCache records the memory capacity real servers report once +// they boot, keyed by server type and image. +// +// The estimate in operator.DefaultVMMemoryOverheadPercent is a single fraction +// standing in for a gap that varies by machine size, so it is wrong everywhere +// by a little. A booted node is not an estimate, and one observation is good for +// every future node of that type on that image. This is the same approach the +// AWS provider takes with its own overhead percent, for the same reason. +// +// State is process-local and deliberately not persisted. Karpenter must be able +// to size a node before any node exists, so a cold cache has to be survivable +// anyway; that being true, a durable store would add a failure mode without +// removing one. A restart simply falls back to the estimate until the next node +// registers. +type discoveredCapacityCache struct { + mu sync.RWMutex + byType map[string]resource.Quantity +} + +func newDiscoveredCapacityCache() *discoveredCapacityCache { + return &discoveredCapacityCache{byType: map[string]resource.Quantity{}} +} + +// discoveredKey scopes an observation to the image it was taken on. The gap +// between advertised and visible memory is set by the guest kernel, so a +// different image can produce a different figure for the same server type, and +// carrying a measurement across an image change would apply a number that was +// never true of the new one. +// +// A node class with no resolved images yet keys on the server type alone. That +// is only reachable before the image controller has run, and it is the same +// bucket such a node class would use consistently. +func discoveredKey(serverType string, nodeClass *apiv1.HCloudNodeClass) string { + if nodeClass == nil || len(nodeClass.Status.ResolvedImages) == 0 { + return serverType + } + ids := make([]string, 0, len(nodeClass.Status.ResolvedImages)) + for _, img := range nodeClass.Status.ResolvedImages { + ids = append(ids, fmt.Sprintf("%s/%d", img.Architecture, img.ImageID)) + } + // Sorted so that a reordering of the same images is the same key. + sort.Strings(ids) + return serverType + "|" + strings.Join(ids, ",") +} + +// get returns the measured capacity for a server type under a node class's +// image, if one has been recorded. +func (c *discoveredCapacityCache) get(serverType string, nodeClass *apiv1.HCloudNodeClass) (resource.Quantity, bool) { + c.mu.RLock() + defer c.mu.RUnlock() + q, ok := c.byType[discoveredKey(serverType, nodeClass)] + return q, ok +} + +// record stores a capacity observed on a registered node, keeping the smallest +// value seen. +// +// Monotonically downward on purpose. Nodes of one type do vary slightly, and the +// consequence of the two directions is not symmetric: too low costs a little +// schedulable memory, too high strands a pod on a node that cannot hold it. A +// genuine increase -- a new image, say -- arrives under a different key, so +// holding the minimum here never pins the cache to a stale low value. +func (c *discoveredCapacityCache) record(serverType string, nodeClass *apiv1.HCloudNodeClass, observed resource.Quantity) bool { + if observed.Sign() <= 0 { + return false + } + key := discoveredKey(serverType, nodeClass) + + c.mu.Lock() + defer c.mu.Unlock() + if existing, ok := c.byType[key]; ok && existing.Cmp(observed) <= 0 { + return false + } + c.byType[key] = observed + return true +} + +// Record stores a capacity measured on a registered node. It reports whether +// this changed what the provider will use, so callers can log first discoveries +// and genuine drops without narrating every node that agrees with the cache. +func (p *Provider) Record(serverType string, nodeClass *apiv1.HCloudNodeClass, observed resource.Quantity) bool { + return p.discovered.record(serverType, nodeClass, observed) +} diff --git a/pkg/providers/instancetype/discovered_test.go b/pkg/providers/instancetype/discovered_test.go new file mode 100644 index 0000000..9ab008d --- /dev/null +++ b/pkg/providers/instancetype/discovered_test.go @@ -0,0 +1,221 @@ +package instancetype + +import ( + "context" + "testing" + + "github.com/hetznercloud/hcloud-go/v2/hcloud" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + + apiv1 "github.com/paperclipinc/karpenter-provider-hetzner/pkg/apis/v1" + "github.com/paperclipinc/karpenter-provider-hetzner/pkg/operator" +) + +func imageNodeClass(images ...apiv1.ResolvedImage) *apiv1.HCloudNodeClass { + return &apiv1.HCloudNodeClass{Status: apiv1.HCloudNodeClassStatus{ResolvedImages: images}} +} + +func TestDiscovered_RecordThenGet(t *testing.T) { + c := newDiscoveredCapacityCache() + nc := imageNodeClass(apiv1.ResolvedImage{Architecture: "x86", ImageID: 42}) + + if _, ok := c.get("cx53", nc); ok { + t.Fatal("expected no entry before recording") + } + if changed := c.record("cx53", nc, resource.MustParse("31337Mi")); !changed { + t.Error("first observation should report a change") + } + got, ok := c.get("cx53", nc) + if !ok { + t.Fatal("expected an entry after recording") + } + if got.String() != "31337Mi" { + t.Errorf("expected 31337Mi, got %s", got.String()) + } +} + +// A larger observation must not raise the cached figure. Nodes of one type vary +// slightly, and raising it would hand pods memory some nodes of that type do not +// have. +func TestDiscovered_KeepsSmallest(t *testing.T) { + c := newDiscoveredCapacityCache() + nc := imageNodeClass(apiv1.ResolvedImage{Architecture: "x86", ImageID: 42}) + + c.record("cx53", nc, resource.MustParse("31337Mi")) + + if changed := c.record("cx53", nc, resource.MustParse("31400Mi")); changed { + t.Error("a larger observation should not change the cache") + } + got, _ := c.get("cx53", nc) + if got.String() != "31337Mi" { + t.Errorf("larger observation overwrote the cache: got %s", got.String()) + } + + if changed := c.record("cx53", nc, resource.MustParse("31300Mi")); !changed { + t.Error("a smaller observation should change the cache") + } + got, _ = c.get("cx53", nc) + if got.String() != "31300Mi" { + t.Errorf("expected the smaller value to win, got %s", got.String()) + } +} + +// The advertised-to-visible gap is set by the guest kernel, so a measurement +// taken on one image says nothing about another. +func TestDiscovered_ScopedToImage(t *testing.T) { + c := newDiscoveredCapacityCache() + oldImage := imageNodeClass(apiv1.ResolvedImage{Architecture: "x86", ImageID: 42}) + newImage := imageNodeClass(apiv1.ResolvedImage{Architecture: "x86", ImageID: 99}) + + c.record("cx53", oldImage, resource.MustParse("31337Mi")) + + if _, ok := c.get("cx53", newImage); ok { + t.Error("a measurement from one image must not apply to another") + } + if _, ok := c.get("cx53", oldImage); !ok { + t.Error("the original image should still resolve") + } +} + +// Resolved images are a set, not a sequence: the same images listed in a +// different order are the same image. +func TestDiscovered_ImageOrderIrrelevant(t *testing.T) { + c := newDiscoveredCapacityCache() + a := imageNodeClass( + apiv1.ResolvedImage{Architecture: "x86", ImageID: 42}, + apiv1.ResolvedImage{Architecture: "arm", ImageID: 43}, + ) + b := imageNodeClass( + apiv1.ResolvedImage{Architecture: "arm", ImageID: 43}, + apiv1.ResolvedImage{Architecture: "x86", ImageID: 42}, + ) + + c.record("cx53", a, resource.MustParse("31337Mi")) + if _, ok := c.get("cx53", b); !ok { + t.Error("reordering the same images produced a different key") + } +} + +func TestDiscovered_RejectsNonPositive(t *testing.T) { + c := newDiscoveredCapacityCache() + nc := imageNodeClass(apiv1.ResolvedImage{Architecture: "x86", ImageID: 42}) + + for _, bad := range []string{"0", "-1Mi"} { + if changed := c.record("cx53", nc, resource.MustParse(bad)); changed { + t.Errorf("observation %q should be rejected", bad) + } + if _, ok := c.get("cx53", nc); ok { + t.Errorf("observation %q was stored", bad) + } + } +} + +// The point of the whole mechanism: a measured node overrides the estimate. +func TestDiscovered_OverridesEstimateInList(t *testing.T) { + st := makeServerType("cx53", hcloud.ArchitectureX86, hcloud.CPUTypeShared, 16, 32, 320, testPricings) + client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} + p := NewProvider(client, operator.DefaultVMMemoryOverheadPercent) + nc := imageNodeClass(apiv1.ResolvedImage{Architecture: "x86", ImageID: 42}) + + before, err := p.List(context.Background(), nc) + if err != nil { + t.Fatal(err) + } + estimated := before[0].Capacity[corev1.ResourceMemory] + // 32Gi less 7.5% is ~30310Mi, short of the 31337Mi a real cx53 reports. + if estimated.Value() >= 31337*1024*1024 { + t.Fatalf("expected the estimate to undershoot reality, got %s", estimated.String()) + } + + p.Record("cx53", nc, resource.MustParse("31337Mi")) + + after, err := p.List(context.Background(), nc) + if err != nil { + t.Fatal(err) + } + measured := after[0].Capacity[corev1.ResourceMemory] + if measured.Value() != 31337*1024*1024 { + t.Errorf("expected the measured capacity to be used, got %s", measured.String()) + } +} + +// A measurement recorded under one node class must not leak into another whose +// images differ, even though both share the catalogue cache. +func TestDiscovered_DoesNotLeakAcrossNodeClasses(t *testing.T) { + st := makeServerType("cx53", hcloud.ArchitectureX86, hcloud.CPUTypeShared, 16, 32, 320, testPricings) + client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} + p := NewProvider(client, operator.DefaultVMMemoryOverheadPercent) + + measured := imageNodeClass(apiv1.ResolvedImage{Architecture: "x86", ImageID: 42}) + other := imageNodeClass(apiv1.ResolvedImage{Architecture: "x86", ImageID: 99}) + + p.Record("cx53", measured, resource.MustParse("31337Mi")) + + types, err := p.List(context.Background(), other) + if err != nil { + t.Fatal(err) + } + got := types[0].Capacity[corev1.ResourceMemory] + if got.Value() == 31337*1024*1024 { + t.Error("a measurement leaked into a node class with a different image") + } +} + +func TestEvictionThreshold(t *testing.T) { + capacity := corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("32Gi"), + corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"), + } + + tests := []struct { + name string + hard map[string]string + wantMemory string + }{ + {name: "absent uses kubelet default", hard: nil, wantMemory: "100Mi"}, + {name: "explicit quantity", hard: map[string]string{"memory.available": "400Mi"}, wantMemory: "400Mi"}, + {name: "percentage of capacity", hard: map[string]string{"memory.available": "10%"}, wantMemory: "3435973836"}, + {name: "unparseable yields nothing", hard: map[string]string{"memory.available": "banana"}, wantMemory: ""}, + {name: "empty falls back to default", hard: map[string]string{"memory.available": ""}, wantMemory: "100Mi"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := evictionThreshold(&apiv1.KubeletConfiguration{EvictionHard: tc.hard}, capacity) + q, ok := got[corev1.ResourceMemory] + if tc.wantMemory == "" { + if ok { + t.Errorf("expected no memory threshold, got %s", q.String()) + } + return + } + if !ok { + t.Fatal("expected a memory threshold") + } + if q.String() != tc.wantMemory { + t.Errorf("expected %s, got %s", tc.wantMemory, q.String()) + } + }) + } +} + +// A negative or malformed reservation must be dropped rather than treated as +// zero, so a typo cannot silently remove a reservation the node really applies. +func TestParseResourceList_DropsInvalid(t *testing.T) { + got := parseResourceList(map[string]string{ + "cpu": "200m", + "memory": "-512Mi", + "pid": "not-a-quantity", + }) + if _, ok := got[corev1.ResourceMemory]; ok { + t.Error("negative memory reservation was kept") + } + if _, ok := got["pid"]; ok { + t.Error("unparseable reservation was kept") + } + cpu, ok := got[corev1.ResourceCPU] + if !ok || cpu.MilliValue() != 200 { + t.Errorf("valid cpu reservation was lost: %v", got) + } +} diff --git a/pkg/providers/instancetype/instancetype.go b/pkg/providers/instancetype/instancetype.go index 8d12747..6ff3eef 100644 --- a/pkg/providers/instancetype/instancetype.go +++ b/pkg/providers/instancetype/instancetype.go @@ -29,34 +29,53 @@ type ServerTypeClient interface { type Provider struct { client ServerTypeClient + // vmMemoryOverheadPercent estimates the gap between a server type's + // advertised memory and what the guest sees, for types no node has yet + // reported. Once one has, discovered holds the measured value instead. + vmMemoryOverheadPercent float64 + mu sync.RWMutex cachedTypes []*cloudprovider.InstanceType cacheExpiry time.Time unavailable *unavailableCache + discovered *discoveredCapacityCache } // NewProvider creates a new instance type provider. -func NewProvider(client ServerTypeClient) *Provider { +func NewProvider(client ServerTypeClient, vmMemoryOverheadPercent float64) *Provider { return &Provider{ - client: client, + client: client, + vmMemoryOverheadPercent: vmMemoryOverheadPercent, unavailable: newUnavailableCache( // 5m: long enough to route around a saturated location, short enough to // retry it soon. TODO: make configurable via operator config if needed. 5 * time.Minute, ), + discovered: newDiscoveredCapacityCache(), } } -// List returns all available InstanceTypes, filtered to those with offerings in the given locations. -// Results are cached for 6 hours. -func (p *Provider) List(ctx context.Context, locations []string) ([]*cloudprovider.InstanceType, error) { +// List returns all available InstanceTypes for a node class, filtered to those +// with offerings in its locations. A nil node class lists every type with no +// location filter and no declared kubelet reservations. +// +// The 6h cache holds only what depends on the hcloud catalogue. Anything that +// depends on the node class -- its kubelet reservations, and the capacity +// measured for its image -- is applied per call, so a node class edit takes +// effect immediately instead of at the next catalogue refresh. +func (p *Provider) List(ctx context.Context, nodeClass *apiv1.HCloudNodeClass) ([]*cloudprovider.InstanceType, error) { + var locations []string + if nodeClass != nil { + locations = nodeClass.Spec.Locations + } + p.mu.RLock() if p.cachedTypes != nil && time.Now().Before(p.cacheExpiry) { cached := p.cachedTypes p.mu.RUnlock() metrics.RecordCacheHit() - return p.applyAvailability(filterByLocations(cached, locations)), nil + return p.resolve(filterByLocations(cached, locations), nodeClass), nil } p.mu.RUnlock() @@ -66,7 +85,7 @@ func (p *Provider) List(ctx context.Context, locations []string) ([]*cloudprovid // Double-check after acquiring write lock. if p.cachedTypes != nil && time.Now().Before(p.cacheExpiry) { metrics.RecordCacheHit() - return p.applyAvailability(filterByLocations(p.cachedTypes, locations)), nil + return p.resolve(filterByLocations(p.cachedTypes, locations), nodeClass), nil } // Cache miss: fetch fresh data from the hcloud API. @@ -79,13 +98,13 @@ func (p *Provider) List(ctx context.Context, locations []string) ([]*cloudprovid types := make([]*cloudprovider.InstanceType, 0, len(serverTypes)) for _, st := range serverTypes { - types = append(types, toInstanceType(st)) + types = append(types, toInstanceType(st, p.vmMemoryOverheadPercent)) } p.cachedTypes = types p.cacheExpiry = time.Now().Add(cacheTTL) - return p.applyAvailability(filterByLocations(types, locations)), nil + return p.resolve(filterByLocations(types, locations), nodeClass), nil } // MarkUnavailable records that a (serverType, location) offering failed with a @@ -96,17 +115,20 @@ func (p *Provider) MarkUnavailable(serverType, location string) { p.unavailable.markUnavailable(serverType, location) } -// applyAvailability returns copies of the given instance types with each -// offering's Available flag computed live from the unavailable cache, so the -// 6h type-catalog cache never bakes in (and thus never staleness-traps) -// availability. +// resolve returns copies of the given instance types with everything that must +// not be baked into the 6h catalogue cache applied fresh: +// +// - each offering's Available flag, from the unavailable cache, so the +// catalogue never staleness-traps availability; +// - memory capacity measured from a registered node of that type and image, +// when one has been seen, in place of the estimate; +// - the overhead implied by this node class's declared kubelet reservations. // -// The returned InstanceType and Offering structs are fresh value-copies, so -// setting Available never mutates the cached entries. Note that nested -// reference fields (Requirements, Capacity, Overhead) are intentionally shared -// with the cache, not deep-copied: callers must treat returned types as -// read-only and must not mutate those maps. -func (p *Provider) applyAvailability(types []*cloudprovider.InstanceType) []*cloudprovider.InstanceType { +// The returned InstanceType and Offering structs are fresh value-copies, and +// Capacity is rebuilt rather than shared because the discovered value differs +// per node class. Requirements stays shared read-only with the cached entry, so +// callers must not mutate it. +func (p *Provider) resolve(types []*cloudprovider.InstanceType, nodeClass *apiv1.HCloudNodeClass) []*cloudprovider.InstanceType { out := make([]*cloudprovider.InstanceType, len(types)) for i, it := range types { offerings := make(cloudprovider.Offerings, len(it.Offerings)) @@ -116,22 +138,33 @@ func (p *Provider) applyAvailability(types []*cloudprovider.InstanceType) []*clo cp.Available = !p.unavailable.isUnavailable(it.Name, zone) offerings[j] = &cp } + + capacity := make(corev1.ResourceList, len(it.Capacity)) + for k, v := range it.Capacity { + capacity[k] = v + } + if measured, ok := p.discovered.get(it.Name, nodeClass); ok { + capacity[corev1.ResourceMemory] = measured + } + // Construct a fresh InstanceType (rather than copying *it) to avoid - // copying the embedded sync.Once (govet copylocks); Requirements/Capacity/ - // Overhead are intentionally shared read-only with the cached entry. + // copying the embedded sync.Once (govet copylocks), which also matters + // because Allocatable() memoises on first call. out[i] = &cloudprovider.InstanceType{ Name: it.Name, Offerings: offerings, Requirements: it.Requirements, - Capacity: it.Capacity, - Overhead: it.Overhead, + Capacity: capacity, + Overhead: overheadFor(nodeClass, capacity), } } return out } -// toInstanceType maps a Hetzner ServerType to a Karpenter InstanceType. -func toInstanceType(st *hcloud.ServerType) *cloudprovider.InstanceType { +// toInstanceType maps a Hetzner ServerType to a Karpenter InstanceType. Only +// node-class-independent facts belong here, since the result is cached across +// every node class; Overhead is left nil and filled in by resolve. +func toInstanceType(st *hcloud.ServerType, vmMemoryOverheadPercent float64) *cloudprovider.InstanceType { arch := "amd64" if st.Architecture == hcloud.ArchitectureARM { arch = "arm64" @@ -156,8 +189,10 @@ func toInstanceType(st *hcloud.ServerType) *cloudprovider.InstanceType { }) } - // Memory: ServerType.Memory is float32 in GB. - memBytes := int64(float64(st.Memory) * 1024 * 1024 * 1024) + // Memory: ServerType.Memory is float32 in GB. Hetzner's figure is what the + // VM is allocated, not what the guest kernel ends up seeing, so hold back an + // estimate of the difference until a real node tells us the true number. + memBytes := memoryWithVMOverhead(int64(float64(st.Memory)*1024*1024*1024), vmMemoryOverheadPercent) // Disk: ServerType.Disk is int in GB. diskBytes := int64(st.Disk) * 1024 * 1024 * 1024 @@ -177,12 +212,6 @@ func toInstanceType(st *hcloud.ServerType) *cloudprovider.InstanceType { corev1.ResourceEphemeralStorage: *resource.NewQuantity(diskBytes, resource.BinarySI), corev1.ResourcePods: *resource.NewQuantity(110, resource.DecimalSI), }, - Overhead: &cloudprovider.InstanceTypeOverhead{ - KubeReserved: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("100m"), - corev1.ResourceMemory: resource.MustParse("100Mi"), - }, - }, } } diff --git a/pkg/providers/instancetype/instancetype_test.go b/pkg/providers/instancetype/instancetype_test.go index 38c3b17..c370c51 100644 --- a/pkg/providers/instancetype/instancetype_test.go +++ b/pkg/providers/instancetype/instancetype_test.go @@ -46,7 +46,7 @@ var testPricings = []hcloud.ServerTypeLocationPricing{ func TestList_NoLocationFilter(t *testing.T) { st := makeServerType("cx11", hcloud.ArchitectureX86, hcloud.CPUTypeShared, 1, 2, 20, testPricings) client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} - p := NewProvider(client) + p := NewProvider(client, 0) types, err := p.List(context.Background(), nil) if err != nil { @@ -63,10 +63,10 @@ func TestList_NoLocationFilter(t *testing.T) { func TestList_LocationFilter(t *testing.T) { st := makeServerType("cx11", hcloud.ArchitectureX86, hcloud.CPUTypeShared, 1, 2, 20, testPricings) client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} - p := NewProvider(client) + p := NewProvider(client, 0) // Only request nbg1; fsn1 offering should be filtered out but type still returned. - types, err := p.List(context.Background(), []string{"nbg1"}) + types, err := p.List(context.Background(), locationsNodeClass("nbg1")) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -81,9 +81,9 @@ func TestList_LocationFilter(t *testing.T) { func TestList_LocationFilterExcludesAll(t *testing.T) { st := makeServerType("cx11", hcloud.ArchitectureX86, hcloud.CPUTypeShared, 1, 2, 20, testPricings) client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} - p := NewProvider(client) + p := NewProvider(client, 0) - types, err := p.List(context.Background(), []string{"hel1"}) // not in pricings + types, err := p.List(context.Background(), locationsNodeClass("hel1")) // not in pricings if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -94,7 +94,7 @@ func TestList_LocationFilterExcludesAll(t *testing.T) { func TestInstanceType_Capacity(t *testing.T) { st := makeServerType("cx21", hcloud.ArchitectureX86, hcloud.CPUTypeShared, 2, 4, 40, testPricings) - it := toInstanceType(st) + it := toInstanceType(st, 0) cpu := it.Capacity[corev1.ResourceCPU] if cpu.Value() != 2 { @@ -121,7 +121,7 @@ func TestInstanceType_Capacity(t *testing.T) { func TestInstanceType_ArchARM(t *testing.T) { st := makeServerType("cax11", hcloud.ArchitectureARM, hcloud.CPUTypeShared, 2, 4, 40, testPricings) - it := toInstanceType(st) + it := toInstanceType(st, 0) archReq := it.Requirements.Get("kubernetes.io/arch") if archReq.Any() != "arm64" { @@ -131,7 +131,7 @@ func TestInstanceType_ArchARM(t *testing.T) { func TestInstanceType_ArchX86(t *testing.T) { st := makeServerType("cx11", hcloud.ArchitectureX86, hcloud.CPUTypeShared, 1, 2, 20, testPricings) - it := toInstanceType(st) + it := toInstanceType(st, 0) archReq := it.Requirements.Get("kubernetes.io/arch") if archReq.Any() != "amd64" { @@ -176,7 +176,7 @@ func TestHourlyNetPrice(t *testing.T) { func TestList_CacheHit(t *testing.T) { st := makeServerType("cx11", hcloud.ArchitectureX86, hcloud.CPUTypeShared, 1, 2, 20, testPricings) client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} - p := NewProvider(client) + p := NewProvider(client, 0) _, _ = p.List(context.Background(), nil) _, _ = p.List(context.Background(), nil) @@ -189,7 +189,7 @@ func TestList_CacheHit(t *testing.T) { func TestList_ReflectsUnavailable(t *testing.T) { st := makeServerType("cx11", hcloud.ArchitectureX86, hcloud.CPUTypeShared, 1, 2, 20, testPricings) client := &mockServerTypeClient{types: []*hcloud.ServerType{st}} - p := NewProvider(client) + p := NewProvider(client, 0) // Before marking: both offerings (nbg1, fsn1) must be available. before, err := p.List(context.Background(), nil)