Skip to content
Open
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
25 changes: 20 additions & 5 deletions kubetest2-gke/deployer/down.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package deployer

import (
"errors"
"fmt"
"sync"

Expand Down Expand Up @@ -55,13 +56,27 @@ func (d *Deployer) Down() error {
klog.V(1).Infof("Deleted %d network firewall rules", numDeletedFWRules)
}

if err := d.TeardownNetwork(); err != nil {
return err
errCleanNat := d.CleanupNat()
if errCleanNat != nil {
klog.Errorf("Error cleaning up NAT: %v", errCleanNat)
}
if err := d.DeleteSubnets(d.retryCount); err != nil {
return err

errTeardownNetwork := d.TeardownNetwork()
if errTeardownNetwork != nil {
klog.Errorf("Error tearing down network: %v", errTeardownNetwork)
}

errDeleteSubnets := d.DeleteSubnets(d.retryCount)
if errDeleteSubnets != nil {
klog.Errorf("Error deleting subnets: %v", errDeleteSubnets)
}

errDeleteNetwork := d.DeleteNetwork()
if errDeleteNetwork != nil {
klog.Errorf("Error deleting network: %v", errDeleteNetwork)
}
return d.DeleteNetwork()

return errors.Join(errCleanFirewalls, errCleanNat, errTeardownNetwork, errDeleteSubnets, errDeleteNetwork)
}

func (d *Deployer) DeleteClusters(retryCount int) {
Expand Down
98 changes: 98 additions & 0 deletions kubetest2-gke/deployer/nat.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
Copyright 2025 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package deployer

import (
"fmt"

"k8s.io/klog/v2"
"sigs.k8s.io/kubetest2/pkg/exec"
)

func (d *Deployer) EnsureNat() error {
if !d.CreateNat {
return nil
}
if d.Network == "default" {
return fmt.Errorf("NAT router should be set manually for the default network")
Comment on lines +30 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So my understanding is that using custom networks for test environments is best practice as it provides isolation and control over resources, like nat and firewalls (reason why in EnsureFirewallRules we've got the same check). Cleaning up test-specific resources on the default network could become difficult/messy

Copy link
Contributor

Choose a reason for hiding this comment

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

the EnsureFirewallRules has

	// Do not modify the firewall rules for the default network
	if d.Network == "default" {
		return nil
	}

so, it is better to fail on validation of the flags that at runtime, specially for these options.

Do we have more clear the use case for enabling the nat creation or are we just trying to map 1 to 1 options?

}
region := regionFromLocation(d.Regions, d.Zones, d.retryCount)
nat := d.getNatName()
hostProject := d.Projects[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

is it always in the first project?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a common assumption throughout the codebase, for example in firewall.go ensureFirewallRulesForMultiProjects:

func (d *Deployer) ensureFirewallRulesForMultiProjects() error {
	hostProject := d.Projects[0]
        ...

But also in multiple places in network.go


if runWithNoOutput(exec.Command("gcloud", "compute", "routers", "describe", nat,
"--project="+hostProject,
"--region="+region,
"--format=value(name)")) != nil {
klog.V(1).Infof("Couldn't describe router '%s', assuming it doesn't exist and creating it", nat)
if err := runWithOutput(exec.Command("gcloud", "compute", "routers", "create", nat,
"--project="+hostProject,
"--network="+d.Network,
"--region="+region)); err != nil {
return fmt.Errorf("error creating NAT router: %w", err)
}
}
// Create this unique NAT configuration only if it does not exist yet.
if runWithNoOutput(exec.Command("gcloud", "compute", "routers", "nats", "describe", nat,
"--project="+hostProject,
"--router="+nat,
"--region="+region,
"--format=value(name)")) != nil {
klog.V(1).Infof("Couldn't describe NAT '%s', assuming it doesn't exist and creating it", nat)
if err := runWithOutput(exec.Command("gcloud", "compute", "routers", "nats", "create", nat,
"--project="+hostProject,
"--router="+nat,
"--region="+region,
"--auto-allocate-nat-external-ips",
"--nat-primary-subnet-ip-ranges")); err != nil {
return fmt.Errorf("error adding NAT to a router: %w", err)
}
}

return nil
}

func (d *Deployer) getNatName() string {
return "nat-router-" + d.Clusters[0]
}

func (d *Deployer) CleanupNat() error {
if !d.CreateNat {
return nil
}
region := regionFromLocation(d.Regions, d.Zones, d.retryCount)
nat := d.getNatName()
hostProject := d.Projects[0]

// Delete NAT router. That will remove NAT configuration as well.
if runWithNoOutput(exec.Command("gcloud", "compute", "routers", "describe", nat,
"--project="+hostProject,
"--region="+region,
"--format=value(name)")) == nil {
klog.V(1).Infof("Found NAT router '%s', deleting", nat)
err := runWithOutput(exec.Command("gcloud", "compute", "routers", "delete", "-q", nat,
"--project="+hostProject,
"--region="+region))
if err != nil {
return fmt.Errorf("error deleting NAT router: %w", err)
}
} else {
klog.V(1).Infof("Found no NAT router '%s', assuming resources are clean", nat)
}

return nil
}
1 change: 1 addition & 0 deletions kubetest2-gke/deployer/options/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ type NetworkOptions struct {
EnableULAInternalIPv6 bool `flag:"~enable-ula-internal-ipv6" desc:"Enable Unique Local IPv6 Addresses (ULA). Adds the --enable-ula-internal-ipv6 flag to the gcloud compute networks create command"`

RemoveNetwork bool `flag:"~remove-network" desc:"At the end of the test remove non-default network that was used by cluster. The 'default' network is never deleted. Defaults to true if not provided."`
CreateNat bool `flag:"~create-nat" desc:"Configure Cloud NAT allowing outbound connections in cluster with private nodes."`
}
3 changes: 3 additions & 0 deletions kubetest2-gke/deployer/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,9 @@ func (d *Deployer) TestSetup() error {
if err := d.EnsureFirewallRules(); err != nil {
return err
}
if err := d.EnsureNat(); err != nil {
return err
}
d.testPrepared = true
return nil
}
Expand Down