Skip to content

Commit 5b2069a

Browse files
committed
feat(commander): run node commands over SSH so pkg/e2e accepts the cluster
The previous commit returned a Cluster with a nil NodeExecutor, and pkg/e2e rejects that outright: provider "commander" returned a cluster without a NodeExecutor That check is deliberate and covered by TestConnectWithProviderValidatesCluster/missing_node_executor, so the answer is to supply an executor rather than to loosen the contract. The NodeExecutor doc comment already describes the shape this should take: "the node InternalIP behind the master for Commander". commanderNodeExecutor resolves the node name to its InternalIP through the Kubernetes API and SSHes to it over the connector's existing route - the bastion when one is configured - as the same user the master is reached with. No infrastructure access beyond what Commander already gives us, and no new SSH plumbing: buildSSHClient was already parameterised by host and user. It opens a client per Exec rather than holding one per node: node commands are occasional in these suites, and a long-lived connection per node touched would keep sessions open for the whole run. Exit codes follow the interface contract - a command that ran and exited non-zero is reported through ExecResult.ExitCode with a nil error; only transport failures are errors. Disks stays nil, which pkg/e2e already handles by substituting a stub that explains itself: Commander hands out a cluster, not the machines under it. Tested: InternalIP selection (preferred over ExternalIP/Hostname), the no-InternalIP and unknown-node errors, and that a resolve failure surfaces before any SSH is attempted. The SSH path itself needs a live cluster and is exercised by the consuming suite. Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
1 parent 63bfd62 commit 5b2069a

3 files changed

Lines changed: 233 additions & 11 deletions

File tree

internal/provisioning/commander/connect_test_cluster.go

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"context"
2121
"fmt"
2222

23+
"k8s.io/client-go/kubernetes"
24+
2325
"github.com/deckhouse/storage-e2e/pkg/clusterprovider"
2426
)
2527

@@ -31,24 +33,49 @@ var _ clusterprovider.Provider = (*commanderProvider)(nil)
3133
//
3234
// The connection itself is the one the legacy pkg/cluster path already used
3335
// through the Connector interface - SSH to the master via the bastion, kubeconfig
34-
// fetched off the master, in-process API tunnel - so this delegates to Connect
35-
// rather than duplicating it.
36+
// fetched off the master, in-process API tunnel - so this reuses the connector
37+
// rather than duplicating any of it.
3638
//
37-
// Nodes and Disks are nil: Commander hands out a cluster, not the infrastructure
38-
// under it, so there is no node-exec transport and no way to attach block
39-
// devices. Suites that need either must run on a provider that offers them (dvp);
40-
// suites that only talk to the Kubernetes API - the common case for a storage
41-
// module whose backend is external - are fully served here.
39+
// Disks stays nil: Commander hands out a cluster, not the infrastructure under
40+
// it, so there is no way to attach block devices. Cluster documents Disks as
41+
// nillable and pkg/e2e substitutes a stub that says so when a suite tries.
4242
func (p *commanderProvider) ConnectTestCluster(ctx context.Context) (*clusterprovider.Cluster, error) {
43-
restConfig, cleanup, err := p.Connect(ctx)
43+
// Detach cancellation: the tunnel must outlive the caller's connect ctx (the
44+
// suite keeps the connection for its whole run); Cleanup tears it down.
45+
ctx = context.WithoutCancel(ctx)
46+
47+
creds, err := p.conf.Resolve()
48+
if err != nil {
49+
return nil, fmt.Errorf("resolve commander credentials: %w", err)
50+
}
51+
conn := newConnector(p.client, p.conf, creds, p.logger)
52+
53+
// The node executor SSHes to nodes as the same user the master is reached
54+
// with, over the same hops.
55+
_, sshUser, err := conn.resolveMaster(ctx)
56+
if err != nil {
57+
return nil, err
58+
}
59+
60+
restConfig, cleanup, err := conn.Connect(ctx)
4461
if err != nil {
4562
return nil, fmt.Errorf("connect to the commander cluster: %w", err)
4663
}
4764

65+
clientset, err := kubernetes.NewForConfig(restConfig)
66+
if err != nil {
67+
cleanup()
68+
return nil, fmt.Errorf("build clientset for node address lookups: %w", err)
69+
}
70+
4871
return &clusterprovider.Cluster{
4972
RESTConfig: restConfig,
50-
Nodes: nil,
51-
Disks: nil,
52-
Cleanup: cleanup,
73+
Nodes: &commanderNodeExecutor{
74+
conn: conn,
75+
resolver: &internalIPResolver{clientset: clientset},
76+
user: sshUser,
77+
},
78+
Disks: nil,
79+
Cleanup: cleanup,
5380
}, nil
5481
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Copyright 2026 Flant JSC
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 commander
18+
19+
import (
20+
"context"
21+
"errors"
22+
"fmt"
23+
24+
cryptossh "golang.org/x/crypto/ssh"
25+
corev1 "k8s.io/api/core/v1"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"k8s.io/client-go/kubernetes"
28+
29+
"github.com/deckhouse/storage-e2e/pkg/clusterprovider"
30+
)
31+
32+
// nodeAddressResolver maps a Kubernetes node name to an address reachable over
33+
// SSH. Commander does not own the infrastructure, so the only address it can
34+
// offer is the one the node reports itself.
35+
type nodeAddressResolver interface {
36+
Resolve(ctx context.Context, nodeName string) (string, error)
37+
}
38+
39+
// internalIPResolver reads the node's InternalIP from the Kubernetes API.
40+
type internalIPResolver struct {
41+
clientset kubernetes.Interface
42+
}
43+
44+
func (r *internalIPResolver) Resolve(ctx context.Context, nodeName string) (string, error) {
45+
node, err := r.clientset.CoreV1().Nodes().Get(ctx, nodeName, metav1.GetOptions{})
46+
if err != nil {
47+
return "", fmt.Errorf("get node %s: %w", nodeName, err)
48+
}
49+
for _, addr := range node.Status.Addresses {
50+
if addr.Type == corev1.NodeInternalIP && addr.Address != "" {
51+
return addr.Address, nil
52+
}
53+
}
54+
return "", fmt.Errorf("node %s reports no InternalIP", nodeName)
55+
}
56+
57+
// commanderNodeExecutor runs commands on test cluster nodes over SSH. The node
58+
// name is resolved to its InternalIP through the Kubernetes API, and the
59+
// connection reuses the connector's route - the bastion when one is configured,
60+
// then the node itself - so it needs no infrastructure access Commander does not
61+
// already give us.
62+
//
63+
// A fresh client per Exec: node commands are occasional in these suites, and
64+
// holding one connection per node for the whole run would keep sessions open
65+
// against every node a scenario ever touched.
66+
type commanderNodeExecutor struct {
67+
conn *connector
68+
resolver nodeAddressResolver
69+
user string
70+
}
71+
72+
var _ clusterprovider.NodeExecutor = (*commanderNodeExecutor)(nil)
73+
74+
func (e *commanderNodeExecutor) Exec(ctx context.Context, nodeName, command string) (clusterprovider.ExecResult, error) {
75+
ip, err := e.resolver.Resolve(ctx, nodeName)
76+
if err != nil {
77+
return clusterprovider.ExecResult{}, err
78+
}
79+
80+
client, err := e.conn.buildSSHClient(ctx, ip, e.user)
81+
if err != nil {
82+
return clusterprovider.ExecResult{}, fmt.Errorf("connect to node %s (%s): %w", nodeName, ip, err)
83+
}
84+
defer func() { _ = client.Close() }()
85+
86+
res, err := client.Exec(ctx, command)
87+
out := clusterprovider.ExecResult{
88+
Stdout: res.Stdout,
89+
Stderr: res.Stderr,
90+
ExitCode: res.ExitCode,
91+
}
92+
// Per the NodeExecutor contract a command that ran and exited non-zero is
93+
// not an error: the exit code carries that. Only transport failures are.
94+
var exitErr *cryptossh.ExitError
95+
if errors.As(err, &exitErr) {
96+
return out, nil
97+
}
98+
if err != nil {
99+
return out, fmt.Errorf("exec on node %s (%s): %w", nodeName, ip, err)
100+
}
101+
return out, nil
102+
}
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* Copyright 2026 Flant JSC
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 commander
18+
19+
import (
20+
"context"
21+
"strings"
22+
"testing"
23+
24+
corev1 "k8s.io/api/core/v1"
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
"k8s.io/client-go/kubernetes/fake"
27+
)
28+
29+
func nodeWithAddresses(name string, addrs ...corev1.NodeAddress) *corev1.Node {
30+
return &corev1.Node{
31+
ObjectMeta: metav1.ObjectMeta{Name: name},
32+
Status: corev1.NodeStatus{Addresses: addrs},
33+
}
34+
}
35+
36+
func TestInternalIPResolver_PicksInternalIP(t *testing.T) {
37+
node := nodeWithAddresses("worker-1",
38+
corev1.NodeAddress{Type: corev1.NodeHostName, Address: "worker-1"},
39+
corev1.NodeAddress{Type: corev1.NodeExternalIP, Address: "203.0.113.7"},
40+
corev1.NodeAddress{Type: corev1.NodeInternalIP, Address: "10.12.4.151"},
41+
)
42+
r := &internalIPResolver{clientset: fake.NewSimpleClientset(node)}
43+
44+
got, err := r.Resolve(context.Background(), "worker-1")
45+
if err != nil {
46+
t.Fatalf("Resolve returned unexpected error: %v", err)
47+
}
48+
if want := "10.12.4.151"; got != want {
49+
t.Errorf("Resolve() = %q, want %q", got, want)
50+
}
51+
}
52+
53+
func TestInternalIPResolver_ErrorsWithoutInternalIP(t *testing.T) {
54+
node := nodeWithAddresses("worker-1",
55+
corev1.NodeAddress{Type: corev1.NodeExternalIP, Address: "203.0.113.7"},
56+
)
57+
r := &internalIPResolver{clientset: fake.NewSimpleClientset(node)}
58+
59+
if _, err := r.Resolve(context.Background(), "worker-1"); err == nil {
60+
t.Fatal("expected an error for a node without an InternalIP, got nil")
61+
} else if !strings.Contains(err.Error(), "no InternalIP") {
62+
t.Errorf("error = %v, want it to mention the missing InternalIP", err)
63+
}
64+
}
65+
66+
func TestInternalIPResolver_ErrorsOnMissingNode(t *testing.T) {
67+
r := &internalIPResolver{clientset: fake.NewSimpleClientset()}
68+
69+
if _, err := r.Resolve(context.Background(), "absent"); err == nil {
70+
t.Fatal("expected an error for an unknown node, got nil")
71+
}
72+
}
73+
74+
// resolverFunc lets a test stub address resolution.
75+
type resolverFunc func(ctx context.Context, nodeName string) (string, error)
76+
77+
func (f resolverFunc) Resolve(ctx context.Context, nodeName string) (string, error) {
78+
return f(ctx, nodeName)
79+
}
80+
81+
// A resolution failure must surface before any SSH is attempted, so the executor
82+
// is usable (and its errors legible) without a reachable node.
83+
func TestCommanderNodeExecutor_SurfacesResolveError(t *testing.T) {
84+
e := &commanderNodeExecutor{
85+
resolver: resolverFunc(func(context.Context, string) (string, error) {
86+
return "", context.DeadlineExceeded
87+
}),
88+
}
89+
90+
if _, err := e.Exec(context.Background(), "worker-1", "true"); err == nil {
91+
t.Fatal("expected the resolve error to surface, got nil")
92+
}
93+
}

0 commit comments

Comments
 (0)