Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ssh: List pending nodes #368

Merged
merged 7 commits into from
Jan 17, 2024
Merged

ssh: List pending nodes #368

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
77d7353
add test for nodes json output
petersutter Jan 16, 2024
6562344
rename to pendingMachine
petersutter Jan 16, 2024
6d3c1fb
initialize struct
petersutter Jan 16, 2024
c5d3f71
add comment why forbidden error is not logged
petersutter Jan 16, 2024
0cd9fc5
support pending machines
petersutter Jan 16, 2024
acebd86
add test
petersutter Jan 16, 2024
89bb9c0
PR feedback: rename to nodeMap
petersutter Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions pkg/cmd/ssh/connect_information.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,23 @@ func NewConnectInformation(
sshPrivateKeyFile PrivateKeyFile,
nodePrivateKeyFiles []PrivateKeyFile,
nodes []corev1.Node,
pendingNodeNames []string,
user string,
) (*ConnectInformation, error) {
var nodeList []Node
nodeSet := make(map[string]Node)
petersutter marked this conversation as resolved.
Show resolved Hide resolved

for _, pendingNodeName := range pendingNodeNames {
nodeSet[pendingNodeName] = Node{
Name: pendingNodeName,
Status: "Unknown",
}
}

for _, node := range nodes {
n := Node{}
n.Name = node.Name
n.Status = "Ready"
n := Node{
Name: node.Name,
Status: "Ready",
}

if !isNodeReady(node) {
n.Status = "Not Ready"
Expand Down Expand Up @@ -124,7 +133,12 @@ func NewConnectInformation(
}
}

nodeList = append(nodeList, n)
nodeSet[n.Name] = n
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea here, very smart.

}

nodeList := make([]Node, 0, len(nodeSet))
for _, node := range nodeSet {
nodeList = append(nodeList, node)
}

return &ConnectInformation{
Expand Down
15 changes: 15 additions & 0 deletions pkg/cmd/ssh/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,11 +647,23 @@ func (o *SSHOptions) Run(f util.Factory) error {

if !o.Interactive {
var nodes []corev1.Node

var pendingNodeNames []string

if nodeHostname == "" {
nodes, err = getNodes(ctx, shootClient)
if err != nil {
return fmt.Errorf("failed to list shoot cluster nodes: %w", err)
}

// We also want to determine if there are nodes that have not yet joined the cluster.
// This is an optional step, as only Gardener operators have access to the Seeds.
// Regular users will receive a 'Forbidden' error when trying to fetch the Machines.
// However, we do not want to log an error message in this case.
pendingNodeNames, err = getNodeNamesFromMachines(ctx, manager, currentTarget)
if err != nil && !apierrors.IsForbidden(err) {
logger.Info("failed to get shoot cluster node names from machines", "err", err)
}
}

connectInformation, err := NewConnectInformation(
Expand All @@ -664,6 +676,7 @@ func (o *SSHOptions) Run(f util.Factory) error {
o.SSHPrivateKeyFile,
nodePrivateKeyFiles,
nodes,
pendingNodeNames,
o.User,
)
if err != nil {
Expand Down Expand Up @@ -835,6 +848,8 @@ func getNodeNamesFromMachinesOrNodes(ctx context.Context, manager target.Manager

nodeNames, err := getNodeNamesFromMachines(ctx, manager, currentTarget)
if err != nil {
// Regular users do not have the permission to fetch the machines.
// However, in this case, we do not want to log an error message. Instead, we will fallback to read the node names.
if !apierrors.IsForbidden(err) {
logger.Info("failed to fetch node names from machine objects", "err", err)
}
Expand Down
46 changes: 31 additions & 15 deletions pkg/cmd/ssh/ssh_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ var _ = Describe("SSH Command", func() {
testShoot *gardencorev1beta1.Shoot
testNode *corev1.Node
testMachine *machinev1alpha1.Machine
pendingMachine *machinev1alpha1.Machine
seedKubeconfigSecret *corev1.Secret
gardenClient client.Client
shootClient client.Client
Expand Down Expand Up @@ -255,7 +256,7 @@ var _ = Describe("SSH Command", func() {
WithStatusSubresource(&operationsv1alpha1.Bastion{}).
Build())

// create a fake shoot cluster with a single node in it
// create a fake shoot cluster with two machines, where one node has already joined the cluster
testNode = &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "node1",
Expand All @@ -272,10 +273,21 @@ var _ = Describe("SSH Command", func() {
ObjectMeta: metav1.ObjectMeta{
Name: "machine1",
Namespace: "shoot--prod1--test-shoot",
Labels: map[string]string{"node": "node1"},
Labels: map[string]string{machinev1alpha1.NodeLabelKey: "node1"},
},
}

pendingMachine = &machinev1alpha1.Machine{
ObjectMeta: metav1.ObjectMeta{
Name: "monitoring1",
Namespace: "shoot--prod1--test-shoot",
Labels: map[string]string{machinev1alpha1.NodeLabelKey: "monitoring1"},
},
}

shootClient = internalfake.NewClientWithObjects(testNode)
seedClient = internalfake.NewClientWithObjects(testMachine, pendingMachine)

streams, _, out, _ = util.NewTestIOStreams()

ctrl = gomock.NewController(GinkgoT())
Expand Down Expand Up @@ -303,14 +315,16 @@ var _ = Describe("SSH Command", func() {

Describe("RunE", func() {
BeforeEach(func() {
seedClientConfig, err := clientcmd.NewClientConfigFromBytes(seedKubeconfigSecret.Data["kubeconfig"])
Expect(err).NotTo(HaveOccurred())
clientProvider.EXPECT().FromClientConfig(gomock.Eq(seedClientConfig)).Return(seedClient, nil).AnyTimes()
petersutter marked this conversation as resolved.
Show resolved Hide resolved

clientProvider.EXPECT().FromClientConfig(gomock.Any()).Return(shootClient, nil).AnyTimes().
Do(func(clientConfig clientcmd.ClientConfig) {
config, err := clientConfig.RawConfig()
Expect(err).NotTo(HaveOccurred())
Expect(config.CurrentContext).To(Equal(testShoot.Namespace + "--" + testShoot.Name + "-" + testShoot.Status.AdvertisedAddresses[0].Name))
})

shootClient = internalfake.NewClientWithObjects(testNode)
})

It("should reject bad options", func() {
Expand Down Expand Up @@ -644,6 +658,19 @@ var _ = Describe("SSH Command", func() {
Expect(info.Bastion.PreferredAddress).To(Equal("0.0.0.0"))
Expect(info.Bastion.SSHPrivateKeyFile).To(Equal(options.SSHPrivateKeyFile))
Expect(info.Bastion.SSHPublicKeyFile).To(Equal(options.SSHPublicKeyFile))
Expect(info.Nodes).To(ConsistOf([]ssh.Node{
{
Name: pendingMachine.Labels[machinev1alpha1.NodeLabelKey],
Status: "Unknown",
},
{
Name: testNode.Name,
Status: "Not Ready",
Address: ssh.Address{
Hostname: nodeHostname,
},
},
}))
Expect(info.NodePrivateKeyFiles).NotTo(BeEmpty())
})

Expand All @@ -669,17 +696,6 @@ var _ = Describe("SSH Command", func() {
manager = targetmocks.NewMockManager(ctrl)
client = gardenclientmocks.NewMockClient(ctrl)

monitoringMachine := &machinev1alpha1.Machine{
ObjectMeta: metav1.ObjectMeta{
Name: "monitoring1",
Namespace: "shoot--prod1--test-shoot",
Labels: map[string]string{"node": "monitoring1"},
},
}

seedClient = internalfake.NewClientWithObjects(testMachine, monitoringMachine)
shootClient = internalfake.NewClientWithObjects(testNode)

factory.ManagerImpl = manager
manager.EXPECT().CurrentTarget().Return(currentTarget, nil)
manager.EXPECT().GardenClient(currentTarget.GardenName()).Return(client, nil)
Expand Down