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

Refactor ingress status IP address #4490

Merged
merged 1 commit into from
Aug 25, 2019
Merged

Refactor ingress status IP address #4490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
64 changes: 37 additions & 27 deletions internal/ingress/status/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,35 +171,12 @@ func NewStatusSyncer(podInfo *k8s.PodInfo, config Config) Syncer {
// runningAddresses returns a list of IP addresses and/or FQDN where the
// ingress controller is currently running
func (s *statusSync) runningAddresses() ([]string, error) {
addrs := []string{}

if s.PublishService != "" {
ns, name, _ := k8s.ParseNameNS(s.PublishService)
svc, err := s.Client.CoreV1().Services(ns).Get(name, metav1.GetOptions{})
if err != nil {
return nil, err
}

if svc.Spec.Type == apiv1.ServiceTypeExternalName {
addrs = append(addrs, svc.Spec.ExternalName)
return addrs, nil
}

for _, ip := range svc.Status.LoadBalancer.Ingress {
if ip.IP == "" {
addrs = append(addrs, ip.Hostname)
} else {
addrs = append(addrs, ip.IP)
}
}

addrs = append(addrs, svc.Spec.ExternalIPs...)
return addrs, nil
if s.PublishStatusAddress != "" {
return []string{s.PublishStatusAddress}, nil
}

if s.PublishStatusAddress != "" {
addrs = append(addrs, s.PublishStatusAddress)
return addrs, nil
if s.PublishService != "" {
return statusAddressFromService(s.PublishService, s.Client)
}

// get information about all the pods running the ingress controller
Expand All @@ -210,6 +187,7 @@ func (s *statusSync) runningAddresses() ([]string, error) {
return nil, err
}

addrs := make([]string, 0)
for _, pod := range pods.Items {
// only Running pods are valid
if pod.Status.Phase != apiv1.PodRunning {
Expand Down Expand Up @@ -346,3 +324,35 @@ func ingressSliceEqual(lhs, rhs []apiv1.LoadBalancerIngress) bool {

return true
}

func statusAddressFromService(service string, kubeClient clientset.Interface) ([]string, error) {
ns, name, _ := k8s.ParseNameNS(service)
svc, err := kubeClient.CoreV1().Services(ns).Get(name, metav1.GetOptions{})
if err != nil {
return nil, err
}

switch svc.Spec.Type {
case apiv1.ServiceTypeExternalName:
return []string{svc.Spec.ExternalName}, nil
case apiv1.ServiceTypeClusterIP:
return []string{svc.Spec.ClusterIP}, nil
case apiv1.ServiceTypeNodePort:
return []string{svc.Spec.ClusterIP}, nil
case apiv1.ServiceTypeLoadBalancer:
addresses := []string{}
for _, ip := range svc.Status.LoadBalancer.Ingress {
if ip.IP == "" {
addresses = append(addresses, ip.Hostname)
} else {
addresses = append(addresses, ip.IP)
}
}

addresses = append(addresses, svc.Spec.ExternalIPs...)

return addresses, nil
}

return nil, fmt.Errorf("unable to extract IP address/es from service %v", service)
}
194 changes: 153 additions & 41 deletions internal/ingress/status/status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package status

import (
"os"
"reflect"
"testing"
"time"

Expand Down Expand Up @@ -373,15 +374,154 @@ func TestKeyfunc(t *testing.T) {
}

func TestRunningAddresessWithPublishService(t *testing.T) {
fk := buildStatusSync()

r, _ := fk.runningAddresses()
if r == nil {
t.Fatalf("returned nil but expected valid []string")
testCases := map[string]struct {
fakeClient *testclient.Clientset
expected []string
errExpected bool
}{
"service type ClusterIP": {
testclient.NewSimpleClientset(
&apiv1.PodList{Items: []apiv1.Pod{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: apiv1.NamespaceDefault,
},
Spec: apiv1.PodSpec{
NodeName: "foo_node",
},
Status: apiv1.PodStatus{
Phase: apiv1.PodRunning,
},
},
},
},
&apiv1.ServiceList{Items: []apiv1.Service{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: apiv1.NamespaceDefault,
},
Spec: apiv1.ServiceSpec{
Type: apiv1.ServiceTypeClusterIP,
ClusterIP: "1.1.1.1",
},
},
},
},
),
[]string{"1.1.1.1"},
false,
},
"service type NodePort": {
testclient.NewSimpleClientset(
&apiv1.ServiceList{Items: []apiv1.Service{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: apiv1.NamespaceDefault,
},
Spec: apiv1.ServiceSpec{
Type: apiv1.ServiceTypeNodePort,
ClusterIP: "1.1.1.1",
},
},
},
},
),
[]string{"1.1.1.1"},
false,
},
"service type ExternalName": {
testclient.NewSimpleClientset(
&apiv1.ServiceList{Items: []apiv1.Service{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: apiv1.NamespaceDefault,
},
Spec: apiv1.ServiceSpec{
Type: apiv1.ServiceTypeExternalName,
ExternalName: "foo.bar",
},
},
},
},
),
[]string{"foo.bar"},
false,
},
"service type LoadBalancer": {
testclient.NewSimpleClientset(
&apiv1.ServiceList{Items: []apiv1.Service{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: apiv1.NamespaceDefault,
},
Spec: apiv1.ServiceSpec{
Type: apiv1.ServiceTypeLoadBalancer,
},
Status: apiv1.ServiceStatus{
LoadBalancer: apiv1.LoadBalancerStatus{
Ingress: []apiv1.LoadBalancerIngress{
{
IP: "10.0.0.1",
},
{
IP: "",
Hostname: "foo",
},
},
},
},
},
},
},
),
[]string{"10.0.0.1", "foo"},
false,
},
"invalid service type": {
testclient.NewSimpleClientset(
&apiv1.ServiceList{Items: []apiv1.Service{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: apiv1.NamespaceDefault,
},
},
},
},
),
nil,
true,
},
}
rl := len(r)
if len(r) != 4 {
t.Errorf("returned %v but expected %v", rl, 4)

for title, tc := range testCases {
t.Run(title, func(t *testing.T) {

fk := buildStatusSync()
fk.Config.Client = tc.fakeClient

ra, err := fk.runningAddresses()
if err != nil {
if tc.errExpected {
return
}

t.Fatalf("%v: unexpected error obtaining running address/es: %v", title, err)
}

if ra == nil {
t.Fatalf("returned nil but expected valid []string")
}

if !reflect.DeepEqual(tc.expected, ra) {
t.Errorf("returned %v but expected %v", ra, tc.expected)
}
})
}
}

Expand All @@ -405,49 +545,21 @@ func TestRunningAddresessWithPods(t *testing.T) {

func TestRunningAddresessWithPublishStatusAddress(t *testing.T) {
fk := buildStatusSync()
fk.PublishService = ""
fk.PublishStatusAddress = "127.0.0.1"

r, _ := fk.runningAddresses()
if r == nil {
ra, _ := fk.runningAddresses()
if ra == nil {
t.Fatalf("returned nil but expected valid []string")
}
rl := len(r)
if len(r) != 1 {
rl := len(ra)
if len(ra) != 1 {
t.Errorf("returned %v but expected %v", rl, 1)
}
rv := r[0]
rv := ra[0]
if rv != "127.0.0.1" {
t.Errorf("returned %v but expected %v", rv, "127.0.0.1")
}
}

/*
TODO: this test requires a refactoring
func TestUpdateStatus(t *testing.T) {
fk := buildStatusSync()
newIPs := buildLoadBalancerIngressByIP()
fk.updateStatus(newIPs)

fooIngress1, err1 := fk.Client.Extensions().Ingresses(apiv1.NamespaceDefault).Get("foo_ingress_1", metav1.GetOptions{})
if err1 != nil {
t.Fatalf("unexpected error")
}
fooIngress1CurIPs := fooIngress1.Status.LoadBalancer.Ingress
if !ingressSliceEqual(fooIngress1CurIPs, newIPs) {
t.Fatalf("returned %v but expected %v", fooIngress1CurIPs, newIPs)
}

fooIngress2, err2 := fk.Client.Extensions().Ingresses(apiv1.NamespaceDefault).Get("foo_ingress_2", metav1.GetOptions{})
if err2 != nil {
t.Fatalf("unexpected error")
}
fooIngress2CurIPs := fooIngress2.Status.LoadBalancer.Ingress
if !ingressSliceEqual(fooIngress2CurIPs, []apiv1.LoadBalancerIngress{}) {
t.Fatalf("returned %v but expected %v", fooIngress2CurIPs, []apiv1.LoadBalancerIngress{})
}
}
*/
func TestSliceToStatus(t *testing.T) {
fkEndpoints := []string{
"10.0.0.1",
Expand Down