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

Configure ginkgo to use the KUBE_BASTION_SSH env variable #16008

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion cmd/kops/get_instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ type renderableCloudInstance struct {
Status string `json:"status"`
Roles []string `json:"roles"`
InternalIP string `json:"internalIP"`
ExternalIP string `json:"externalIP"`
InstanceGroup string `json:"instanceGroup"`
MachineType string `json:"machineType"`
State string `json:"state"`
Expand Down Expand Up @@ -182,6 +183,9 @@ func instanceOutputTable(instances []*cloudinstances.CloudInstance, out io.Write
t.AddColumn("INTERNAL-IP", func(i *cloudinstances.CloudInstance) string {
return i.PrivateIP
})
t.AddColumn("EXTERNAL-IP", func(i *cloudinstances.CloudInstance) string {
return i.ExternalIP
})
t.AddColumn("INSTANCE-GROUP", func(i *cloudinstances.CloudInstance) string {
return i.CloudInstanceGroup.HumanName
})
Expand All @@ -192,7 +196,7 @@ func instanceOutputTable(instances []*cloudinstances.CloudInstance, out io.Write
return string(i.State)
})

columns := []string{"ID", "NODE-NAME", "STATUS", "ROLES", "STATE", "INTERNAL-IP", "INSTANCE-GROUP", "MACHINE-TYPE"}
columns := []string{"ID", "NODE-NAME", "STATUS", "ROLES", "STATE", "INTERNAL-IP", "EXTERNAL-IP", "INSTANCE-GROUP", "MACHINE-TYPE"}
return t.Render(instances, out, columns...)
}

Expand Down Expand Up @@ -220,6 +224,7 @@ func asRenderable(instances []*cloudinstances.CloudInstance) []*renderableCloudI
Status: ci.Status,
Roles: ci.Roles,
InternalIP: ci.PrivateIP,
ExternalIP: ci.ExternalIP,
InstanceGroup: ci.CloudInstanceGroup.HumanName,
MachineType: ci.MachineType,
State: string(ci.State),
Expand Down
6 changes: 4 additions & 2 deletions pkg/cloudinstances/cloud_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,16 @@ type CloudInstance struct {
Node *v1.Node
// CloudInstanceGroup is the managing CloudInstanceGroup
CloudInstanceGroup *CloudInstanceGroup
// Status indicates if the instance has joined the cluster and if it needs any updates.
// Status indicates the state of instance is in as reported by the Cloud APIs
Status string
// Roles are the roles the instance have.
Roles []string
// MachineType is the hardware resource class of the instance.
MachineType string
// Private IP is the private ip address of the instance.
PrivateIP string
// State is in which state the instance is in
// External IP is the public ip address of the instance.
ExternalIP string
// State indicates if the instance has joined the cluster and if it needs any updates.
State State
}
297 changes: 238 additions & 59 deletions tests/e2e/go.mod

Large diffs are not rendered by default.

2,988 changes: 738 additions & 2,250 deletions tests/e2e/go.sum

Large diffs are not rendered by default.

23 changes: 23 additions & 0 deletions tests/e2e/pkg/kops/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"k8s.io/klog/v2"
api "k8s.io/kops/pkg/apis/kops/v1alpha2"
"k8s.io/kops/pkg/cloudinstances"
"sigs.k8s.io/kubetest2/pkg/exec"
)

Expand Down Expand Up @@ -61,6 +62,28 @@ func GetCluster(kopsBinary, clusterName string, env []string) (*api.Cluster, err
return cluster, nil
}

// GetCluster will retrieve the specified Cluster from the state store.
func GetInstances(kopsBinary, clusterName string, env []string) ([]*cloudinstances.CloudInstance, error) {
args := []string{
kopsBinary, "get", "instances", "--name", clusterName, "-ojson",
}
c := exec.Command(args[0], args[1:]...)
c.SetEnv(env...)
var stdout bytes.Buffer
c.SetStdout(&stdout)
var stderr bytes.Buffer
c.SetStderr(&stderr)
if err := c.Run(); err != nil {
klog.Warningf("failed to run %s; stderr=%s", strings.Join(args, " "), stderr.String())
return nil, fmt.Errorf("error querying instances from %s: %w", strings.Join(args, " "), err)
}
var instances []*cloudinstances.CloudInstance
if err := json.Unmarshal(stdout.Bytes(), &instances); err != nil {
return nil, fmt.Errorf("error parsing instance groups json: %w", err)
}
return instances, nil
}

// GetInstanceGroups will retrieve the instance groups for the specified Cluster from the state store.
func GetInstanceGroups(kopsBinary, clusterName string, env []string) ([]*api.InstanceGroup, error) {
args := []string{
Expand Down
38 changes: 25 additions & 13 deletions tests/e2e/pkg/tester/tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,6 @@ func parseKubeconfig(jsonPath string) (string, error) {
return s, nil
}

// The --host flag was required in the kubernetes e2e tests, until https://github.com/kubernetes/kubernetes/pull/87030
// We can likely drop this when we drop support / testing for k8s 1.17
func (t *Tester) addHostFlag() error {
server, err := parseKubeconfig(".clusters[0].cluster.server")
if err != nil {
return err
}
klog.Infof("Adding --host=%s", server)
t.TestArgs += " --host=" + server
return nil
}

// hasFlag detects if the specified flag has been passed in the args
func hasFlag(args string, flag string) bool {
for _, arg := range strings.Split(args, " ") {
Expand Down Expand Up @@ -141,6 +129,30 @@ func (t *Tester) getKopsInstanceGroups() ([]*api.InstanceGroup, error) {
return igs, nil
}

// k/k tests assume the API Server in the kubeconfig is the IP of the control plane node instead of a Loadbalancer.
// Some tests try to SSH to the API Server IP which fails for kops as the LB doesn't expose port 22
func (t *Tester) addKubeBastion() error {
cluster, err := t.getKopsCluster()
if err != nil {
return err
}
if cluster.Spec.LegacyCloudProvider == "gce" {
instances, err := kops.GetInstances("kops", cluster.Name, nil)
if err != nil {
return err
}
for _, instance := range instances {
for _, role := range instance.Roles {
if role == "control-plane" && instance.ExternalIP != "" {
t.Env = append(t.Env, fmt.Sprintf("KUBE_SSH_BASTION=%s", instance.ExternalIP))
Copy link
Member

Choose a reason for hiding this comment

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

For a three-node control plane we would set KUBE_SSH_BASTION three times?

Copy link
Member Author

Choose a reason for hiding this comment

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

Forgot about that, we need just one IP.

break
}
}
}
}
return nil
}

func (t *Tester) addProviderFlag() error {
if hasFlag(t.TestArgs, "provider") {
return nil
Expand Down Expand Up @@ -422,7 +434,7 @@ func (t *Tester) execute() error {
return err
}

if err := t.addHostFlag(); err != nil {
if err := t.addKubeBastion(); err != nil {
return err
}

Expand Down
37 changes: 35 additions & 2 deletions upup/pkg/fi/cloudup/gce/instancegroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"k8s.io/klog/v2"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/cloudinstances"
"k8s.io/kops/upup/pkg/fi"
)

// DeleteGroup deletes a cloud of instances controlled by an Instance Group Manager
Expand Down Expand Up @@ -171,13 +172,18 @@ func getCloudGroups(c GCECloud, cluster *kops.Cluster, instancegroups []*kops.In

for _, i := range instances {
id := i.Instance
name := LastComponent(id)
instance, err := c.Compute().Instances().Get(project, zoneName, name)
if err != nil {
return nil, fmt.Errorf("error getting Instance: %v", err)
}
cm := &cloudinstances.CloudInstance{
ID: id,
ID: instance.SelfLink,
CloudInstanceGroup: g,
}
addCloudInstanceData(cm, instance)

// Try first by provider ID
name := LastComponent(id)
providerID := "gce://" + project + "/" + zoneName + "/" + name
node := nodesByProviderID[providerID]

Expand Down Expand Up @@ -257,3 +263,30 @@ func matchInstanceGroup(mig *compute.InstanceGroupManager, c *kops.Cluster, inst
}
return matches[0], nil
}

func addCloudInstanceData(cm *cloudinstances.CloudInstance, instance *compute.Instance) {
cm.MachineType = LastComponent(instance.MachineType)
cm.Status = instance.Status
if instance.Status == "RUNNING" {
cm.State = cloudinstances.CloudInstanceStatusUpToDate
}
isControlPlane := false
for k := range instance.Labels {
if !strings.HasPrefix(k, GceLabelNameRolePrefix) {
continue
}
role := strings.TrimPrefix(k, GceLabelNameRolePrefix)
if role == "master" || role == "control-plane" {
isControlPlane = true
} else {
cm.Roles = append(cm.Roles, role)
cm.PrivateIP = fi.ValueOf(&instance.NetworkInterfaces[0].NetworkIP)
cm.ExternalIP = fi.ValueOf(&instance.NetworkInterfaces[0].AccessConfigs[0].NatIP)
}
}
if isControlPlane {
cm.Roles = append(cm.Roles, "control-plane")
cm.PrivateIP = fi.ValueOf(&instance.NetworkInterfaces[0].NetworkIP)
cm.ExternalIP = fi.ValueOf(&instance.NetworkInterfaces[0].AccessConfigs[0].NatIP)
}
Copy link
Member

Choose a reason for hiding this comment

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

Why does this avoid setting the PrivateIP and ExternalIP when there is no role label? Why not unconditionally set those regardless of the role label?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't, look at 282 to 284.

}