Skip to content

Commit 8f98000

Browse files
committed
- Gosec fix
- Cleanup daemonset once all host are registered
1 parent f1f423c commit 8f98000

File tree

5 files changed

+107
-42
lines changed

5 files changed

+107
-42
lines changed

controllers/lxd_initializer_ds.go

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/spectrocloud/cluster-api-provider-maas/pkg/maas/scope"
1818
"github.com/spectrocloud/cluster-api-provider-maas/pkg/util/trust"
19+
"github.com/spectrocloud/cluster-api-provider-maas/pkg/util"
1920

2021
// embed template
2122
_ "embed"
@@ -66,6 +67,7 @@ func (r *MaasClusterReconciler) ensureLXDInitializerDS(ctx context.Context, clus
6667
// Gate: derive desired CP count from MaasCloudConfig; fallback to KCP
6768
desiredCP := int32(1)
6869
readyByKCP := int32(0)
70+
6971
// Prefer MaasCloudConfig.spec.machinePoolConfig[].size where isControlPlane=true (sum)
7072
{
7173
mccList := &unstructured.UnstructuredList{}
@@ -99,7 +101,7 @@ func (r *MaasClusterReconciler) ensureLXDInitializerDS(ctx context.Context, clus
99101
}
100102
}
101103
if sum > 0 {
102-
desiredCP = int32(sum)
104+
desiredCP = util.SafeInt64ToInt32(sum)
103105
break
104106
}
105107
}
@@ -115,10 +117,10 @@ func (r *MaasClusterReconciler) ensureLXDInitializerDS(ctx context.Context, clus
115117
if len(kcpList.Items) > 0 {
116118
item := kcpList.Items[0]
117119
if v, found, _ := unstructured.NestedInt64(item.Object, "spec", "replicas"); found && v > 0 {
118-
desiredCP = int32(v)
120+
desiredCP = util.SafeInt64ToInt32(v)
119121
}
120122
if v, found, _ := unstructured.NestedInt64(item.Object, "status", "readyReplicas"); found && v >= 0 {
121-
readyByKCP = int32(v)
123+
readyByKCP = util.SafeInt64ToInt32(v)
122124
}
123125
}
124126
}
@@ -139,7 +141,7 @@ func (r *MaasClusterReconciler) ensureLXDInitializerDS(ctx context.Context, clus
139141
}
140142
}
141143
// Proceed when CP nodes are present and Ready, regardless of KCP readyReplicas
142-
if int32(len(nodeList.Items)) < desiredCP || int32(ready) < desiredCP {
144+
if int64(len(nodeList.Items)) < int64(desiredCP) || int64(ready) < int64(desiredCP) {
143145
r.Log.Info("Not enough control-plane nodes present/ready yet; skipping DS for now", "desiredCP", desiredCP, "readyByKCP", readyByKCP, "nodeList", len(nodeList.Items), "ready", ready)
144146
// Not enough control-plane nodes present/ready yet; skip DS for now
145147
return nil
@@ -166,6 +168,33 @@ func (r *MaasClusterReconciler) ensureLXDInitializerDS(ctx context.Context, clus
166168
return fmt.Errorf("failed to ensure LXD initializer RBAC: %v", err)
167169
}
168170

171+
// Short-circuit deletion: if all control-plane nodes are labeled initialized, delete DS
172+
shortCircuitNodes := &corev1.NodeList{}
173+
shortCircuitSelector := labels.SelectorFromSet(labels.Set{
174+
"node-role.kubernetes.io/control-plane": "",
175+
})
176+
if err := remoteClient.List(ctx, shortCircuitNodes, &client.ListOptions{LabelSelector: shortCircuitSelector}); err == nil && len(shortCircuitNodes.Items) > 0 {
177+
// Count how many CP nodes are initialized
178+
initCount := 0
179+
for _, n := range shortCircuitNodes.Items {
180+
if n.Labels != nil && n.Labels["lxdhost.cluster.com/initialized"] == "true" {
181+
initCount++
182+
}
183+
}
184+
// Delete DS only when we see at least desiredCP control-plane nodes
185+
// AND desiredCP of them are initialized
186+
if int64(len(shortCircuitNodes.Items)) >= int64(desiredCP) && int64(initCount) >= int64(desiredCP) {
187+
// Delete existing DSs and return
188+
shortCircuitDSList := &appsv1.DaemonSetList{}
189+
if err := remoteClient.List(ctx, shortCircuitDSList, client.InNamespace(dsNamespace), client.MatchingLabels{"app": dsName}); err == nil {
190+
for _, ds := range shortCircuitDSList.Items {
191+
_ = remoteClient.Delete(ctx, &ds)
192+
}
193+
}
194+
return nil
195+
}
196+
}
197+
169198
// pull LXD config
170199
cfg := clusterScope.GetLXDConfig()
171200
sb := cfg.StorageBackend

controllers/templates/lxd_initializer_ds.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ spec:
7373
mountPropagation: HostToContainer
7474
containers:
7575
- name: lxd-initializer
76-
image: us-east1-docker.pkg.dev/spectro-images/dev/amit/cluster-api/lxd-initializer:v0.6.1-spectro-4.0.0-dev-15102025-01
76+
image: us-east1-docker.pkg.dev/spectro-images/dev/amit/cluster-api/lxd-initializer:v0.6.1-spectro-4.0.0-dev-16102025-01
7777
imagePullPolicy: Always
7878
securityContext:
7979
privileged: true

lxd-initializer/lxd-initializer-daemonset.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ spec:
2020
hostPID: true
2121
containers:
2222
- name: lxd-initializer
23-
image: us-east1-docker.pkg.dev/spectro-images/dev/amit/cluster-api/lxd-initializer:v0.6.1-spectro-4.0.0-dev-15102025-01
23+
image: us-east1-docker.pkg.dev/spectro-images/dev/amit/cluster-api/lxd-initializer:v0.6.1-spectro-4.0.0-dev-16102025-01
2424
imagePullPolicy: Always
2525
securityContext:
2626
privileged: true

lxd-initializer/lxd-initializer.go

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ package main
22

33
import (
44
"context"
5+
crand "crypto/rand"
56
"flag"
67
"fmt"
78
"log"
8-
"math/rand"
99
"net"
1010
"net/url"
1111
"os"
@@ -245,48 +245,30 @@ func registerWithMAAS(maasEndpoint, maasAPIKey, systemID, nodeIP, trustPassword,
245245
ctx := context.Background()
246246
client := maasclient.NewAuthenticatedClientSet(maasEndpoint, maasAPIKey)
247247

248-
// Guard 1: Verify the MAAS machine identified by systemID owns nodeIP (or derive power IP)
249-
m, err := client.Machines().Machine(systemID).Get(ctx)
250-
if err != nil {
251-
return fmt.Errorf("get machine %s: %w", systemID, err)
252-
}
253-
hostIP := nodeIP
254-
owns := false
255-
for _, ip := range m.IPAddresses() {
256-
if ip.String() == nodeIP {
257-
owns = true
258-
break
259-
}
260-
}
261-
if !owns {
262-
ips := m.IPAddresses()
263-
if len(ips) == 0 {
264-
return fmt.Errorf("ownership check failed: system-id %s has no IPs; cannot register", systemID)
265-
}
266-
hostIP = ips[0].String()
267-
}
268-
269248
// Idempotency/conflict checks via API for speed
270249
hosts, err := client.VMHosts().List(ctx, nil)
271250
if err != nil {
272251
return fmt.Errorf("list vm hosts: %w", err)
273252
}
274-
// Align with manual flow: bare IP for power_address
275-
wantHost := hostIP
253+
// Align with manual flow: bare IP for power_address (still used for create below)
254+
wantHost := nodeIP
255+
// Strict guards: rely only on name and system-id for idempotency
256+
expectedName := hostName
276257
for _, h := range hosts {
277-
if normalizeHost(h.PowerAddress()) == normalizeHost(wantHost) {
278-
if h.HostSystemID() != "" && h.HostSystemID() != systemID {
279-
return fmt.Errorf("conflict: existing VM host %s uses %s mapped to %s", h.Name(), wantHost, h.HostSystemID())
258+
// 1) Exact name match → idempotent or conflict
259+
if h.Name() == expectedName {
260+
if h.HostSystemID() == "" || h.HostSystemID() == systemID {
261+
log.Printf("MAAS VM host already present (name=%s, system-id=%s); skipping re-registration", h.Name(), h.HostSystemID())
262+
return nil
280263
}
281-
if h.Name() != hostName {
282-
return fmt.Errorf("conflict: power_address %s already used by VM host %s (name mismatch)", wantHost, h.Name())
283-
}
284-
log.Printf("MAAS VM host already present: name=%s power_address=%s", h.Name(), h.PowerAddress())
285-
return nil
264+
return fmt.Errorf("conflict: VM host %q belongs to system-id %s (expected %s)", h.Name(), h.HostSystemID(), systemID)
286265
}
287-
if h.Name() == hostName && h.HostSystemID() != "" && h.HostSystemID() != systemID {
288-
return fmt.Errorf("conflict: existing VM host %s belongs to system-id %s (expected %s)", h.Name(), h.HostSystemID(), systemID)
266+
// 2) Same system already registered under a different name → idempotent
267+
if h.HostSystemID() == systemID {
268+
log.Printf("MAAS VM host for system-id=%s already exists under name=%s; skipping re-registration", systemID, h.Name())
269+
return nil
289270
}
271+
// 3) Non-matching entry → ignore
290272
}
291273

292274
// Prefer MAAS CLI for creation to match manual success path
@@ -561,6 +543,20 @@ func main() {
561543
actionStr = "both" // Default to both init and register
562544
}
563545

546+
// Early exit: if node already marked initialized, skip all work
547+
if nodeName != "" {
548+
if client, err := getKubernetesClient(); err == nil {
549+
if node, gerr := client.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{}); gerr == nil {
550+
if node.Labels != nil {
551+
if node.Labels[LXDHostInitializedLabel] == LabelValueTrue {
552+
log.Printf("Node %s already labeled %s=true; skipping initializer", nodeName, LXDHostInitializedLabel)
553+
return
554+
}
555+
}
556+
}
557+
}
558+
}
559+
564560
// Perform actions based on the specified action
565561
if actionStr == "init" || actionStr == "both" {
566562
// Initialize LXD
@@ -632,8 +628,16 @@ func main() {
632628
}
633629
actualDelaySec := delaySec
634630
if jitterSec > 0 {
635-
rand.Seed(time.Now().UnixNano())
636-
actualDelaySec += rand.Intn(jitterSec + 1)
631+
// crypto-strong jitter in [0, jitterSec]
632+
var rb [8]byte
633+
if _, err := crand.Read(rb[:]); err == nil {
634+
// Convert to uint64, then mod
635+
rnd := int(rb[0])
636+
if jitterSec > 0 {
637+
rnd = rnd % (jitterSec + 1)
638+
}
639+
actualDelaySec += rnd
640+
}
637641
}
638642
delay := time.Duration(actualDelaySec) * time.Second
639643
log.Printf("LXD init complete; staggering for %v before host registration (index=%d/%d, per=%ds, cap=%ds, jitter<=%ds)", delay, nodeIndex, nodeCount, perNodeSec, maxCapSec, jitterSec)

pkg/util/utils.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package util
18+
19+
import (
20+
"math"
21+
)
22+
23+
// SafeInt64ToInt32 safely converts int64 to int32 with overflow protection
24+
func SafeInt64ToInt32(value int64) int32 {
25+
if value > math.MaxInt32 {
26+
return math.MaxInt32
27+
}
28+
if value < math.MinInt32 {
29+
return math.MinInt32
30+
}
31+
return int32(value)
32+
}

0 commit comments

Comments
 (0)