From 0400871df9996ed5e7583eb7fcf470351ce5c43f Mon Sep 17 00:00:00 2001 From: Justin SB Date: Mon, 5 Nov 2018 22:19:05 -0500 Subject: [PATCH 1/2] e2e: block all master addresses This way we can be sure that the kubelet can't communicate with the master, even if falls-back to the internal/external IP (which seems to be the case with DNS) Issue #56787 --- test/e2e/apps/network_partition.go | 20 +++++++++++----- test/e2e/framework/networking_utils.go | 10 +++++--- test/e2e/framework/util.go | 21 ++++++++++++----- test/e2e/network/firewall.go | 24 +++++++++++--------- test/e2e/scheduling/taint_based_evictions.go | 10 +++++--- 5 files changed, 56 insertions(+), 29 deletions(-) diff --git a/test/e2e/apps/network_partition.go b/test/e2e/apps/network_partition.go index 8fbebc96802..7d20149d3ec 100644 --- a/test/e2e/apps/network_partition.go +++ b/test/e2e/apps/network_partition.go @@ -198,10 +198,12 @@ var _ = SIGDescribe("Network Partition [Disruptive] [Slow]", func() { By(fmt.Sprintf("Block traffic from node %s to the master", node.Name)) host, err := framework.GetNodeExternalIP(&node) framework.ExpectNoError(err) - master := framework.GetMasterAddress(c) + masterAddresses := framework.GetAllMasterAddresses(c) defer func() { By(fmt.Sprintf("Unblock traffic from node %s to the master", node.Name)) - framework.UnblockNetwork(host, master) + for _, masterAddress := range masterAddresses { + framework.UnblockNetwork(host, masterAddress) + } if CurrentGinkgoTestDescription().Failed { return @@ -214,7 +216,9 @@ var _ = SIGDescribe("Network Partition [Disruptive] [Slow]", func() { } }() - framework.BlockNetwork(host, master) + for _, masterAddress := range masterAddresses { + framework.BlockNetwork(host, masterAddress) + } By("Expect to observe node and pod status change from Ready to NotReady after network partition") expectNodeReadiness(false, newNode) @@ -576,10 +580,12 @@ var _ = SIGDescribe("Network Partition [Disruptive] [Slow]", func() { By(fmt.Sprintf("Block traffic from node %s to the master", node.Name)) host, err := framework.GetNodeExternalIP(&node) framework.ExpectNoError(err) - master := framework.GetMasterAddress(c) + masterAddresses := framework.GetAllMasterAddresses(c) defer func() { By(fmt.Sprintf("Unblock traffic from node %s to the master", node.Name)) - framework.UnblockNetwork(host, master) + for _, masterAddress := range masterAddresses { + framework.UnblockNetwork(host, masterAddress) + } if CurrentGinkgoTestDescription().Failed { return @@ -589,7 +595,9 @@ var _ = SIGDescribe("Network Partition [Disruptive] [Slow]", func() { expectNodeReadiness(true, newNode) }() - framework.BlockNetwork(host, master) + for _, masterAddress := range masterAddresses { + framework.BlockNetwork(host, masterAddress) + } By("Expect to observe node and pod status change from Ready to NotReady after network partition") expectNodeReadiness(false, newNode) diff --git a/test/e2e/framework/networking_utils.go b/test/e2e/framework/networking_utils.go index 7f507a8390f..a8f8831d2c5 100644 --- a/test/e2e/framework/networking_utils.go +++ b/test/e2e/framework/networking_utils.go @@ -952,7 +952,7 @@ func TestUnderTemporaryNetworkFailure(c clientset.Interface, ns string, node *v1 if err != nil { Failf("Error getting node external ip : %v", err) } - master := GetMasterAddress(c) + masterAddresses := GetAllMasterAddresses(c) By(fmt.Sprintf("block network traffic from node %s to the master", node.Name)) defer func() { // This code will execute even if setting the iptables rule failed. @@ -960,14 +960,18 @@ func TestUnderTemporaryNetworkFailure(c clientset.Interface, ns string, node *v1 // had been inserted. (yes, we could look at the error code and ssh error // separately, but I prefer to stay on the safe side). By(fmt.Sprintf("Unblock network traffic from node %s to the master", node.Name)) - UnblockNetwork(host, master) + for _, masterAddress := range masterAddresses { + UnblockNetwork(host, masterAddress) + } }() Logf("Waiting %v to ensure node %s is ready before beginning test...", resizeNodeReadyTimeout, node.Name) if !WaitForNodeToBe(c, node.Name, v1.NodeReady, true, resizeNodeReadyTimeout) { Failf("Node %s did not become ready within %v", node.Name, resizeNodeReadyTimeout) } - BlockNetwork(host, master) + for _, masterAddress := range masterAddresses { + BlockNetwork(host, masterAddress) + } Logf("Waiting %v for node %s to be not ready after simulated network failure", resizeNodeNotReadyTimeout, node.Name) if !WaitForNodeToBe(c, node.Name, v1.NodeReady, false, resizeNodeNotReadyTimeout) { diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 1e7f09319a8..3dca2a881e8 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -4934,19 +4934,28 @@ func getMaster(c clientset.Interface) Address { return master } -// GetMasterAddress returns the hostname/external IP/internal IP as appropriate for e2e tests on a particular provider -// which is the address of the interface used for communication with the kubelet. -func GetMasterAddress(c clientset.Interface) string { +// GetAllMasterAddresses returns all IP addresses on which the kubelet can reach the master. +// It may return internal and external IPs, even if we expect for +// e.g. internal IPs to be used (issue #56787), so that we can be +// sure to block the master fully during tests. +func GetAllMasterAddresses(c clientset.Interface) []string { master := getMaster(c) + + var ips sets.String switch TestContext.Provider { case "gce", "gke": - return master.externalIP + if master.externalIP != "" { + ips.Insert(master.externalIP) + } + if master.internalIP != "" { + ips.Insert(master.internalIP) + } case "aws": - return awsMasterIP + ips.Insert(awsMasterIP) default: Failf("This test is not supported for provider %s and should be disabled", TestContext.Provider) } - return "" + return ips.List() } // GetNodeExternalIP returns node external IP concatenated with port 22 for ssh diff --git a/test/e2e/network/firewall.go b/test/e2e/network/firewall.go index f5c98b70bb2..a4014ab1218 100644 --- a/test/e2e/network/firewall.go +++ b/test/e2e/network/firewall.go @@ -173,16 +173,18 @@ var _ = SIGDescribe("Firewall rule", func() { By("Checking well known ports on master and nodes are not exposed externally") nodeAddrs := framework.NodeAddresses(nodes, v1.NodeExternalIP) Expect(len(nodeAddrs)).NotTo(BeZero()) - masterAddr := framework.GetMasterAddress(cs) - flag, _ := framework.TestNotReachableHTTPTimeout(masterAddr, ports.InsecureKubeControllerManagerPort, gce.FirewallTestTcpTimeout) - Expect(flag).To(BeTrue()) - flag, _ = framework.TestNotReachableHTTPTimeout(masterAddr, ports.SchedulerPort, gce.FirewallTestTcpTimeout) - Expect(flag).To(BeTrue()) - flag, _ = framework.TestNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletPort, gce.FirewallTestTcpTimeout) - Expect(flag).To(BeTrue()) - flag, _ = framework.TestNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletReadOnlyPort, gce.FirewallTestTcpTimeout) - Expect(flag).To(BeTrue()) - flag, _ = framework.TestNotReachableHTTPTimeout(nodeAddrs[0], ports.ProxyStatusPort, gce.FirewallTestTcpTimeout) - Expect(flag).To(BeTrue()) + masterAddresses := framework.GetAllMasterAddresses(cs) + for _, masterAddr := range masterAddresses { + flag, _ := framework.TestNotReachableHTTPTimeout(masterAddr, ports.InsecureKubeControllerManagerPort, gce.FirewallTestTcpTimeout) + Expect(flag).To(BeTrue()) + flag, _ = framework.TestNotReachableHTTPTimeout(masterAddr, ports.SchedulerPort, gce.FirewallTestTcpTimeout) + Expect(flag).To(BeTrue()) + flag, _ = framework.TestNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletPort, gce.FirewallTestTcpTimeout) + Expect(flag).To(BeTrue()) + flag, _ = framework.TestNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletReadOnlyPort, gce.FirewallTestTcpTimeout) + Expect(flag).To(BeTrue()) + flag, _ = framework.TestNotReachableHTTPTimeout(nodeAddrs[0], ports.ProxyStatusPort, gce.FirewallTestTcpTimeout) + Expect(flag).To(BeTrue()) + } }) }) diff --git a/test/e2e/scheduling/taint_based_evictions.go b/test/e2e/scheduling/taint_based_evictions.go index 23de100749b..612a8a77d0c 100644 --- a/test/e2e/scheduling/taint_based_evictions.go +++ b/test/e2e/scheduling/taint_based_evictions.go @@ -128,12 +128,14 @@ var _ = SIGDescribe("TaintBasedEvictions [Serial]", func() { // host, err = framework.GetNodeInternalIP(&node) // } framework.ExpectNoError(err) - master := framework.GetMasterAddress(cs) + masterAddresses := framework.GetAllMasterAddresses(cs) taint := newUnreachableNoExecuteTaint() defer func() { By(fmt.Sprintf("Unblocking traffic from node %s to the master", node.Name)) - framework.UnblockNetwork(host, master) + for _, masterAddress := range masterAddresses { + framework.UnblockNetwork(host, masterAddress) + } if CurrentGinkgoTestDescription().Failed { framework.Failf("Current e2e test has failed, so return from here.") @@ -147,7 +149,9 @@ var _ = SIGDescribe("TaintBasedEvictions [Serial]", func() { framework.ExpectNoError(err) }() - framework.BlockNetwork(host, master) + for _, masterAddress := range masterAddresses { + framework.BlockNetwork(host, masterAddress) + } By(fmt.Sprintf("Expecting to see node %q becomes NotReady", nodeName)) if !framework.WaitForNodeToBeNotReady(cs, nodeName, time.Minute*3) { From c136a99bf25ab886eb68fd36ce23ea6c6e3f7af1 Mon Sep 17 00:00:00 2001 From: Justin SB Date: Tue, 6 Nov 2018 13:04:50 -0500 Subject: [PATCH 2/2] e2e: Better error reporting in firewall test Clearly report the particular ip/port that failed. --- test/e2e/framework/util.go | 2 +- test/e2e/network/firewall.go | 35 ++++++++++++++++++++++------------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 3dca2a881e8..b58e8fdb4f2 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -4941,7 +4941,7 @@ func getMaster(c clientset.Interface) Address { func GetAllMasterAddresses(c clientset.Interface) []string { master := getMaster(c) - var ips sets.String + ips := sets.NewString() switch TestContext.Provider { case "gce", "gke": if master.externalIP != "" { diff --git a/test/e2e/network/firewall.go b/test/e2e/network/firewall.go index a4014ab1218..d46ff96c352 100644 --- a/test/e2e/network/firewall.go +++ b/test/e2e/network/firewall.go @@ -18,6 +18,7 @@ package network import ( "fmt" + "time" "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -172,19 +173,27 @@ var _ = SIGDescribe("Firewall rule", func() { By("Checking well known ports on master and nodes are not exposed externally") nodeAddrs := framework.NodeAddresses(nodes, v1.NodeExternalIP) - Expect(len(nodeAddrs)).NotTo(BeZero()) - masterAddresses := framework.GetAllMasterAddresses(cs) - for _, masterAddr := range masterAddresses { - flag, _ := framework.TestNotReachableHTTPTimeout(masterAddr, ports.InsecureKubeControllerManagerPort, gce.FirewallTestTcpTimeout) - Expect(flag).To(BeTrue()) - flag, _ = framework.TestNotReachableHTTPTimeout(masterAddr, ports.SchedulerPort, gce.FirewallTestTcpTimeout) - Expect(flag).To(BeTrue()) - flag, _ = framework.TestNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletPort, gce.FirewallTestTcpTimeout) - Expect(flag).To(BeTrue()) - flag, _ = framework.TestNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletReadOnlyPort, gce.FirewallTestTcpTimeout) - Expect(flag).To(BeTrue()) - flag, _ = framework.TestNotReachableHTTPTimeout(nodeAddrs[0], ports.ProxyStatusPort, gce.FirewallTestTcpTimeout) - Expect(flag).To(BeTrue()) + if len(nodeAddrs) == 0 { + framework.Failf("did not find any node addresses") } + + masterAddresses := framework.GetAllMasterAddresses(cs) + for _, masterAddress := range masterAddresses { + assertNotReachableHTTPTimeout(masterAddress, ports.InsecureKubeControllerManagerPort, gce.FirewallTestTcpTimeout) + assertNotReachableHTTPTimeout(masterAddress, ports.SchedulerPort, gce.FirewallTestTcpTimeout) + } + assertNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletPort, gce.FirewallTestTcpTimeout) + assertNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletReadOnlyPort, gce.FirewallTestTcpTimeout) + assertNotReachableHTTPTimeout(nodeAddrs[0], ports.ProxyStatusPort, gce.FirewallTestTcpTimeout) }) }) + +func assertNotReachableHTTPTimeout(ip string, port int, timeout time.Duration) { + unreachable, err := framework.TestNotReachableHTTPTimeout(ip, port, timeout) + if err != nil { + framework.Failf("Unexpected error checking for reachability of %s:%d: %v", ip, port, err) + } + if !unreachable { + framework.Failf("Was unexpectedly able to reach %s:%d", ip, port) + } +}