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.

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

test(e2e): simplify cluster parameter extraction #5742

Merged
merged 9 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 54 additions & 1 deletion e2e/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/json"
"errors"
"fmt"
"strings"
"sync"
"testing"
"time"
Expand All @@ -17,7 +18,11 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v6"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/tools/clientcmd"
)

var (
Expand All @@ -34,6 +39,12 @@ var (
clusterAzureNetworkOnce sync.Once
)

type ClusterParams struct {
CACert []byte
BootstrapToken string
FQDN string
}

type Cluster struct {
Model *armcontainerservice.ManagedCluster
Kube *Kubeclient
Expand Down Expand Up @@ -127,15 +138,57 @@ func prepareCluster(ctx context.Context, t *testing.T, cluster *armcontainerserv
return nil, fmt.Errorf("collect garbage vmss: %w", err)
}

clusterParams, err := extractClusterParameters(ctx, t, kube, cluster)
if err != nil {
return nil, fmt.Errorf("extracting cluster parameters: %w", err)
}

return &Cluster{
Model: cluster,
Kube: kube,
SubnetID: subnetID,
Maintenance: maintenance,
ClusterParams: extractClusterParameters(ctx, t, kube),
ClusterParams: clusterParams,
}, nil
}

func extractClusterParameters(ctx context.Context, t *testing.T, kube *Kubeclient, cluster *armcontainerservice.ManagedCluster) (*ClusterParams, error) {
resp, err := config.Azure.AKS.ListClusterAdminCredentials(ctx, config.ResourceGroupName, *cluster.Name, nil)
Copy link
Contributor

@lilypan26 lilypan26 Feb 4, 2025

Choose a reason for hiding this comment

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

nit: ik we always set it, but should we include a nil check for cluster.Name

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the cluster object comes from a call to AKS.Get/CreateOrUpdate, so it shouldn't ever be the case that we make it this far in test execution and Name is nil without an error having previously occurring

Copy link
Contributor

Choose a reason for hiding this comment

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

the result will be the same whether you check it for nil or not - a test will fail

Copy link
Contributor

Choose a reason for hiding this comment

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

A kube client is already passed to this function.
Does it mean we already loaded credentials before? Are we doing it second time?

The change makes sense, just wonder if we can optimize it a one step further

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah good point, and unfortunately I saw this after I pressed the merge button :D

if err != nil {
return nil, fmt.Errorf("listing cluster admin credentials: %w", err)
}
kubeconfig, err := clientcmd.Load(resp.Kubeconfigs[0].Value)
if err != nil {
return nil, fmt.Errorf("loading cluster kubeconfig: %w", err)
}
clusterConfig := kubeconfig.Clusters[*cluster.Name]
if clusterConfig == nil {
return nil, fmt.Errorf("cluster kubeconfig missing configuration for %s", *cluster.Name)
}
return &ClusterParams{
CACert: clusterConfig.CertificateAuthorityData,
BootstrapToken: getBootstrapToken(ctx, t, kube),
FQDN: *cluster.Properties.Fqdn,
}, nil
}

func getBootstrapToken(ctx context.Context, t *testing.T, kube *Kubeclient) string {
secrets, err := kube.Typed.CoreV1().Secrets("kube-system").List(ctx, metav1.ListOptions{})
require.NoError(t, err)
secret := func() *corev1.Secret {
for _, secret := range secrets.Items {
if strings.HasPrefix(secret.Name, "bootstrap-token-") {
return &secret
}
}
t.Fatal("could not find secret with bootstrap-token- prefix")
return nil
}()
id := secret.Data["token-id"]
token := secret.Data["token-secret"]
return fmt.Sprintf("%s.%s", id, token)
}

func hash(cluster *armcontainerservice.ManagedCluster) string {
jsonData, err := json.Marshal(cluster)
if err != nil {
Expand Down
71 changes: 3 additions & 68 deletions e2e/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,10 @@ import (
"context"
"fmt"
"strings"
"testing"

"github.com/Azure/agentbaker/e2e/config"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/tools/remotecommand"
)
Expand All @@ -32,66 +29,6 @@ func (r podExecResult) String() string {
`, r.exitCode, r.stderr.String(), r.stdout.String())
}

type ClusterParams struct {
CACert []byte
BootstrapToken string
FQDN string
APIServerCert []byte
ClientKey []byte
}

func extractClusterParameters(ctx context.Context, t *testing.T, kube *Kubeclient) *ClusterParams {
pod, err := kube.GetHostNetworkDebugPod(ctx, t)
require.NoError(t, err)

execResult, err := execOnPrivilegedPod(ctx, kube, pod.Namespace, pod.Name, "cat /var/lib/kubelet/bootstrap-kubeconfig")
require.NoError(t, err)

bootstrapConfig := execResult.stdout.Bytes()

server, err := extractKeyValuePair("server", string(bootstrapConfig))
require.NoError(t, err)
tokens := strings.Split(server, ":")
if len(tokens) != 3 {
t.Fatalf("expected 3 tokens from fqdn %q, got %d", server, len(tokens))
}
fqdn := tokens[1][2:]

caCert, err := execOnPrivilegedPod(ctx, kube, pod.Namespace, pod.Name, "cat /etc/kubernetes/certs/ca.crt")
require.NoError(t, err)

cmdAPIServer, err := execOnPrivilegedPod(ctx, kube, pod.Namespace, pod.Name, "cat /etc/kubernetes/certs/apiserver.crt")
require.NoError(t, err)

clientKey, err := execOnPrivilegedPod(ctx, kube, pod.Namespace, pod.Name, "cat /etc/kubernetes/certs/client.key")
require.NoError(t, err)

return &ClusterParams{
CACert: caCert.stdout.Bytes(),
BootstrapToken: getBootstrapToken(ctx, t, kube),
FQDN: fqdn,
APIServerCert: cmdAPIServer.stdout.Bytes(),
ClientKey: clientKey.stdout.Bytes(),
}
}

func getBootstrapToken(ctx context.Context, t *testing.T, kube *Kubeclient) string {
secrets, err := kube.Typed.CoreV1().Secrets("kube-system").List(ctx, metav1.ListOptions{})
require.NoError(t, err)
secret := func() *corev1.Secret {
for _, secret := range secrets.Items {
if strings.HasPrefix(secret.Name, "bootstrap-token-") {
return &secret
}
}
t.Fatal("could not find secret with bootstrap-token- prefix")
return nil
}()
id := secret.Data["token-id"]
token := secret.Data["token-secret"]
return fmt.Sprintf("%s.%s", id, token)
}

func sshKeyName(vmPrivateIP string) string {
return fmt.Sprintf("sshkey%s", strings.ReplaceAll(vmPrivateIP, ".", ""))

Expand Down Expand Up @@ -133,12 +70,10 @@ func execScriptOnVm(ctx context.Context, s *Scenario, vmPrivateIP, jumpboxPodNam
interpreter = "powershell"
scriptFileName = fmt.Sprintf("script_file_%s.ps1", identifier)
remoteScriptFileName = fmt.Sprintf("c:/%s", scriptFileName)
break
default:
interpreter = "bash"
scriptFileName = fmt.Sprintf("script_file_%s.sh", identifier)
remoteScriptFileName = scriptFileName
break
}

steps := []string{
Expand All @@ -153,7 +88,7 @@ func execScriptOnVm(ctx context.Context, s *Scenario, vmPrivateIP, jumpboxPodNam

joinedSteps := strings.Join(steps, " && ")

s.T.Log(fmt.Sprintf("Executing script %[1]s using %[2]s:\n---START-SCRIPT---\n%[3]s\n---END-SCRIPT---\n", scriptFileName, interpreter, script.script))
s.T.Logf("Executing script %[1]s using %[2]s:\n---START-SCRIPT---\n%[3]s\n---END-SCRIPT---\n", scriptFileName, interpreter, script.script)

kube := s.Runtime.Cluster.Kube
execResult, err := execOnPrivilegedPod(ctx, kube, defaultNamespace, jumpboxPodName, joinedSteps)
Expand Down Expand Up @@ -240,9 +175,9 @@ func unprivilegedCommandArray() []string {
func logSSHInstructions(s *Scenario) {
result := "SSH Instructions:"
if !config.Config.KeepVMSS {
result += fmt.Sprintf(" (VM will be automatically deleted after the test finishes, set KEEP_VMSS=true to preserve it or pause the test with a breakpoint before the test finishes)")
result += " (VM will be automatically deleted after the test finishes, set KEEP_VMSS=true to preserve it or pause the test with a breakpoint before the test finishes)"
}
result += fmt.Sprintf("\n========================\n")
result += "\n========================\n"
result += fmt.Sprintf("az account set --subscription %s\n", config.Config.SubscriptionID)
result += fmt.Sprintf("az aks get-credentials --resource-group %s --name %s --overwrite-existing\n", config.ResourceGroupName, *s.Runtime.Cluster.Model.Name)
result += fmt.Sprintf(`kubectl exec -it %s -- bash -c "chroot /proc/1/root /bin/bash -c '%s'"`, s.Runtime.DebugHostPod, sshString(s.Runtime.VMPrivateIP))
Expand Down
10 changes: 5 additions & 5 deletions e2e/node_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ func getBaseNBC(t *testing.T, cluster *Cluster, vhd *config.Image) *datamodel.No

if vhd.Distro.IsWindowsDistro() {
nbc = baseTemplateWindows(t, config.Config.Location)
cert := cluster.Kube.clientCertificate()
nbc.ContainerService.Properties.CertificateProfile.ClientCertificate = cert
nbc.ContainerService.Properties.CertificateProfile.APIServerCertificate = string(cluster.ClusterParams.APIServerCert)
nbc.ContainerService.Properties.CertificateProfile.ClientPrivateKey = string(cluster.ClusterParams.ClientKey)

// these aren't needed since we use TLS bootstrapping instead, though windows bootstrapping expects non-empty values
nbc.ContainerService.Properties.CertificateProfile.ClientCertificate = "none"
nbc.ContainerService.Properties.CertificateProfile.ClientPrivateKey = "none"

nbc.ContainerService.Properties.ClusterID = *cluster.Model.ID
nbc.SubscriptionID = config.Config.SubscriptionID
nbc.ResourceGroupName = *cluster.Model.Properties.NodeResourceGroup
Expand All @@ -41,7 +42,6 @@ func getBaseNBC(t *testing.T, cluster *Cluster, vhd *config.Image) *datamodel.No
}

nbc.ContainerService.Properties.CertificateProfile.CaCertificate = string(cluster.ClusterParams.CACert)

nbc.KubeletClientTLSBootstrapToken = &cluster.ClusterParams.BootstrapToken
nbc.ContainerService.Properties.HostedMasterProfile.FQDN = cluster.ClusterParams.FQDN
nbc.ContainerService.Properties.AgentPoolProfiles[0].Distro = vhd.Distro
Expand Down
Loading