Skip to content

Commit

Permalink
Merge pull request #1071 from skmatti/metrics-fix-1.9
Browse files Browse the repository at this point in the history 
Cherrypick #1065[User specified Static Global IPs] into release-1.9
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Apr 7, 2020
2 parents 78be425 + 01004f8 commit dc867e5
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 50 deletions.
85 changes: 55 additions & 30 deletions pkg/metrics/features.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,30 @@ const (
// certificate for the Ingress controller to use.
preSharedCertKey = "ingress.gcp.kubernetes.io/pre-shared-cert"
managedCertKey = "networking.gke.io/managed-certificates"
// StaticIPNameKey tells the Ingress controller to use a specific GCE
// static ip for its forwarding rules. If specified, the Ingress controller
// assigns the static ip by this name to the forwarding rules of the given
// Ingress. The controller *does not* manage this ip, it is the users
// responsibility to create/delete it.
StaticIPNameKey = "kubernetes.io/ingress.global-static-ip-name"
// staticIPKey is the annotation key used by controller to record GCP static ip.
staticIPKey = "ingress.kubernetes.io/static-ip"
// SSLCertKey is the annotation key used by controller to record GCP ssl cert.
SSLCertKey = "ingress.kubernetes.io/ssl-cert"

ingress = feature("Ingress")
externalIngress = feature("ExternalIngress")
internalIngress = feature("InternalIngress")
httpEnabled = feature("HTTPEnabled")
hostBasedRouting = feature("HostBasedRouting")
pathBasedRouting = feature("PathBasedRouting")
tlsTermination = feature("TLSTermination")
secretBasedCertsForTLS = feature("SecretBasedCertsForTLS")
preSharedCertsForTLS = feature("PreSharedCertsForTLS")
managedCertsForTLS = feature("ManagedCertsForTLS")
staticGlobalIP = feature("StaticGlobalIP")
ingress = feature("Ingress")
externalIngress = feature("ExternalIngress")
internalIngress = feature("InternalIngress")
httpEnabled = feature("HTTPEnabled")
hostBasedRouting = feature("HostBasedRouting")
pathBasedRouting = feature("PathBasedRouting")
tlsTermination = feature("TLSTermination")
secretBasedCertsForTLS = feature("SecretBasedCertsForTLS")
preSharedCertsForTLS = feature("PreSharedCertsForTLS")
managedCertsForTLS = feature("ManagedCertsForTLS")
staticGlobalIP = feature("StaticGlobalIP")
managedStaticGlobalIP = feature("ManagedStaticGlobalIP")
specifiedStaticGlobalIP = feature("SpecifiedStaticGlobalIP")

servicePort = feature("L7LBServicePort")
externalServicePort = feature("L7XLBServicePort")
Expand Down Expand Up @@ -144,32 +154,47 @@ func featuresForIngress(ing *v1beta1.Ingress) []feature {
}

// SSL certificate based features.
sslConfigured := false
if val, ok := ingAnnotations[preSharedCertKey]; ok {
klog.V(6).Infof("Specified pre-shared certs for ingress %s: %v", ingKey, val)
sslConfigured = true
features = append(features, preSharedCertsForTLS)
}
if val, ok := ingAnnotations[managedCertKey]; ok {
klog.V(6).Infof("Specified google managed certs for ingress %s: %v", ingKey, val)
sslConfigured = true
features = append(features, managedCertsForTLS)
}
if hasSecretBasedCerts(ing) {
sslConfigured = true
features = append(features, secretBasedCertsForTLS)
}
if sslConfigured {
klog.V(6).Infof("TLS termination is configured for ingress %s", ingKey)
// If user specifies a SSL certificate and controller does not add SSL certificate
// annotation on ingress, then SSL cert is invalid in some sense. Ignore reporting
// these SSL certificates.
if cert, ok := ingAnnotations[SSLCertKey]; ok {
klog.V(6).Infof("Configured SSL certificate for ingress %s: %v", ingKey, cert)
features = append(features, tlsTermination)

certSpecified := false
if val, ok := ingAnnotations[preSharedCertKey]; ok {
klog.V(6).Infof("Specified pre-shared certs for ingress %s: %v", ingKey, val)
certSpecified = true
features = append(features, preSharedCertsForTLS)
}
if val, ok := ingAnnotations[managedCertKey]; ok {
klog.V(6).Infof("Specified google managed certs for ingress %s: %v", ingKey, val)
certSpecified = true
features = append(features, managedCertsForTLS)
}
if hasSecretBasedCerts(ing) {
certSpecified = true
features = append(features, secretBasedCertsForTLS)
}
if !certSpecified {
klog.Errorf("Unexpected TLS termination(cert: %s) for ingress %s", cert, ingKey)
}
}

// Both user specified and ingress controller managed global static ips are reported.
if val, ok := ingAnnotations[staticIPKey]; ok && val != "" {
klog.V(6).Infof("Specified static for ingress %s: %s", ingKey, val)
klog.V(6).Infof("Static IP for ingress %s: %s", ingKey, val)
features = append(features, staticGlobalIP)

// Check if user specified static ip annotation exists.
if val, ok = ingAnnotations[StaticIPNameKey]; ok {
klog.V(6).Infof("User specified static IP for ingress %s: %s", ingKey, val)
features = append(features, specifiedStaticGlobalIP)
} else {
features = append(features, managedStaticGlobalIP)
}
}
klog.V(4).Infof("Features for ingress %s/%s: %v", ing.Namespace, ing.Name, features)
klog.V(4).Infof("Features for ingress %s: %v", ingKey, features)
return features
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/metrics/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,8 @@ func initializeCounts() (map[feature]int, map[feature]int) {
preSharedCertsForTLS: 0,
managedCertsForTLS: 0,
staticGlobalIP: 0,
managedStaticGlobalIP: 0,
specifiedStaticGlobalIP: 0,
neg: 0,
cloudCDN: 0,
cloudArmor: 0,
Expand Down
104 changes: 84 additions & 20 deletions pkg/metrics/metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ var (
Name: "ingress6",
Annotations: map[string]string{
preSharedCertKey: "pre-shared-cert1,pre-shared-cert2",
SSLCertKey: "pre-shared-cert1,pre-shared-cert2",
},
},
Spec: v1beta1.IngressSpec{
Expand All @@ -287,7 +288,7 @@ var (
},
},
[]feature{ingress, externalIngress, httpEnabled,
preSharedCertsForTLS, tlsTermination},
tlsTermination, preSharedCertsForTLS},
[]utils.ServicePort{testServicePorts[0]},
[]feature{servicePort, externalServicePort, cloudCDN,
cookieAffinity, cloudArmor, backendConnectionDraining},
Expand All @@ -300,6 +301,7 @@ var (
Name: "ingress7",
Annotations: map[string]string{
managedCertKey: "managed-cert1,managed-cert2",
SSLCertKey: "managed-cert1,managed-cert2",
},
},
Spec: v1beta1.IngressSpec{
Expand All @@ -311,7 +313,7 @@ var (
},
},
[]feature{ingress, externalIngress, httpEnabled,
managedCertsForTLS, tlsTermination},
tlsTermination, managedCertsForTLS},
[]utils.ServicePort{testServicePorts[0]},
[]feature{servicePort, externalServicePort, cloudCDN,
cookieAffinity, cloudArmor, backendConnectionDraining},
Expand All @@ -325,6 +327,7 @@ var (
Annotations: map[string]string{
preSharedCertKey: "pre-shared-cert1,pre-shared-cert2",
managedCertKey: "managed-cert1,managed-cert2",
SSLCertKey: "pre-shared-cert1,pre-shared-cert2,managed-cert1,managed-cert2",
},
},
Spec: v1beta1.IngressSpec{
Expand All @@ -336,7 +339,7 @@ var (
},
},
[]feature{ingress, externalIngress, httpEnabled,
preSharedCertsForTLS, managedCertsForTLS, tlsTermination},
tlsTermination, preSharedCertsForTLS, managedCertsForTLS},
[]utils.ServicePort{testServicePorts[0]},
[]feature{servicePort, externalServicePort, cloudCDN,
cookieAffinity, cloudArmor, backendConnectionDraining},
Expand All @@ -349,6 +352,7 @@ var (
Name: "ingress9",
Annotations: map[string]string{
preSharedCertKey: "pre-shared-cert1,pre-shared-cert2",
SSLCertKey: "pre-shared-cert1,pre-shared-cert2",
},
},
Spec: v1beta1.IngressSpec{
Expand Down Expand Up @@ -379,7 +383,7 @@ var (
},
},
[]feature{ingress, externalIngress, httpEnabled, hostBasedRouting,
pathBasedRouting, preSharedCertsForTLS, secretBasedCertsForTLS, tlsTermination},
pathBasedRouting, tlsTermination, preSharedCertsForTLS, secretBasedCertsForTLS},
[]utils.ServicePort{testServicePorts[1]},
[]feature{servicePort, externalServicePort, neg, cloudIAP,
clientIPAffinity, backendTimeout, customRequestHeaders},
Expand All @@ -392,6 +396,7 @@ var (
Name: "ingress10",
Annotations: map[string]string{
preSharedCertKey: "pre-shared-cert1,pre-shared-cert2",
SSLCertKey: "pre-shared-cert1,pre-shared-cert2",
staticIPKey: "10.0.1.2",
},
},
Expand All @@ -404,7 +409,7 @@ var (
},
},
[]feature{ingress, externalIngress, httpEnabled,
preSharedCertsForTLS, tlsTermination, staticGlobalIP},
tlsTermination, preSharedCertsForTLS, staticGlobalIP, managedStaticGlobalIP},
[]utils.ServicePort{testServicePorts[0]},
[]feature{servicePort, externalServicePort, cloudCDN,
cookieAffinity, cloudArmor, backendConnectionDraining},
Expand Down Expand Up @@ -450,6 +455,52 @@ var (
[]feature{servicePort, internalServicePort, neg, cloudIAP,
cookieAffinity, backendConnectionDraining},
},
{
"non-existent pre-shared cert",
&v1beta1.Ingress{
ObjectMeta: v1.ObjectMeta{
Namespace: defaultNamespace,
Name: "ingress12",
Annotations: map[string]string{
preSharedCertKey: "pre-shared-cert1",
},
},
Spec: v1beta1.IngressSpec{
Backend: &v1beta1.IngressBackend{
ServiceName: "dummy-service",
ServicePort: intstr.FromInt(80),
},
Rules: []v1beta1.IngressRule{},
},
},
[]feature{ingress, externalIngress, httpEnabled},
[]utils.ServicePort{},
nil,
},
{
"user specified global static IP",
&v1beta1.Ingress{
ObjectMeta: v1.ObjectMeta{
Namespace: defaultNamespace,
Name: "ingress13",
Annotations: map[string]string{
StaticIPNameKey: "user-spec-static-ip",
staticIPKey: "user-spec-static-ip",
},
},
Spec: v1beta1.IngressSpec{
Backend: &v1beta1.IngressBackend{
ServiceName: "dummy-service",
ServicePort: intstr.FromInt(80),
},
Rules: []v1beta1.IngressRule{},
},
},
[]feature{ingress, externalIngress, httpEnabled,
staticGlobalIP, specifiedStaticGlobalIP},
[]utils.ServicePort{},
nil,
},
}
)

Expand Down Expand Up @@ -505,6 +556,7 @@ func TestComputeIngressMetrics(t *testing.T) {
NewIngressState(ingressStates[0].ing, ingressStates[0].svcPorts),
NewIngressState(ingressStates[1].ing, ingressStates[1].svcPorts),
NewIngressState(ingressStates[3].ing, ingressStates[3].svcPorts),
NewIngressState(ingressStates[13].ing, ingressStates[13].svcPorts),
},
map[feature]int{
backendConnectionDraining: 0,
Expand All @@ -515,17 +567,19 @@ func TestComputeIngressMetrics(t *testing.T) {
cloudIAP: 0,
cookieAffinity: 0,
customRequestHeaders: 0,
externalIngress: 3,
httpEnabled: 2,
externalIngress: 4,
httpEnabled: 3,
hostBasedRouting: 1,
ingress: 3,
ingress: 4,
internalIngress: 0,
managedCertsForTLS: 0,
managedStaticGlobalIP: 0,
neg: 0,
pathBasedRouting: 0,
preSharedCertsForTLS: 0,
secretBasedCertsForTLS: 0,
staticGlobalIP: 0,
specifiedStaticGlobalIP: 1,
staticGlobalIP: 1,
tlsTermination: 0,
},
map[feature]int{
Expand All @@ -550,6 +604,7 @@ func TestComputeIngressMetrics(t *testing.T) {
NewIngressState(ingressStates[1].ing, ingressStates[1].svcPorts),
NewIngressState(ingressStates[3].ing, ingressStates[3].svcPorts),
NewIngressState(ingressStates[11].ing, ingressStates[11].svcPorts),
NewIngressState(ingressStates[13].ing, ingressStates[13].svcPorts),
},
map[feature]int{
backendConnectionDraining: 1,
Expand All @@ -560,17 +615,19 @@ func TestComputeIngressMetrics(t *testing.T) {
cloudIAP: 1,
cookieAffinity: 1,
customRequestHeaders: 0,
externalIngress: 3,
httpEnabled: 3,
externalIngress: 4,
httpEnabled: 4,
hostBasedRouting: 2,
ingress: 4,
ingress: 5,
internalIngress: 1,
managedCertsForTLS: 0,
managedStaticGlobalIP: 0,
neg: 1,
pathBasedRouting: 1,
preSharedCertsForTLS: 0,
secretBasedCertsForTLS: 0,
staticGlobalIP: 0,
specifiedStaticGlobalIP: 1,
staticGlobalIP: 1,
tlsTermination: 0,
},
map[feature]int{
Expand All @@ -596,6 +653,7 @@ func TestComputeIngressMetrics(t *testing.T) {
NewIngressState(ingressStates[6].ing, ingressStates[6].svcPorts),
NewIngressState(ingressStates[8].ing, ingressStates[8].svcPorts),
NewIngressState(ingressStates[10].ing, ingressStates[10].svcPorts),
NewIngressState(ingressStates[12].ing, ingressStates[12].svcPorts),
},
map[feature]int{
backendConnectionDraining: 4,
Expand All @@ -606,16 +664,18 @@ func TestComputeIngressMetrics(t *testing.T) {
cloudIAP: 1,
cookieAffinity: 4,
customRequestHeaders: 1,
externalIngress: 5,
httpEnabled: 5,
externalIngress: 6,
httpEnabled: 6,
hostBasedRouting: 1,
ingress: 5,
ingress: 6,
internalIngress: 0,
managedCertsForTLS: 1,
managedStaticGlobalIP: 1,
neg: 1,
pathBasedRouting: 1,
preSharedCertsForTLS: 3,
secretBasedCertsForTLS: 0,
specifiedStaticGlobalIP: 0,
staticGlobalIP: 1,
tlsTermination: 3,
},
Expand Down Expand Up @@ -649,6 +709,8 @@ func TestComputeIngressMetrics(t *testing.T) {
NewIngressState(ingressStates[9].ing, ingressStates[9].svcPorts),
NewIngressState(ingressStates[10].ing, ingressStates[10].svcPorts),
NewIngressState(ingressStates[11].ing, ingressStates[11].svcPorts),
NewIngressState(ingressStates[12].ing, ingressStates[12].svcPorts),
NewIngressState(ingressStates[13].ing, ingressStates[13].svcPorts),
},
map[feature]int{
backendConnectionDraining: 7,
Expand All @@ -659,17 +721,19 @@ func TestComputeIngressMetrics(t *testing.T) {
cloudIAP: 4,
cookieAffinity: 7,
customRequestHeaders: 3,
externalIngress: 11,
httpEnabled: 11,
externalIngress: 13,
httpEnabled: 13,
hostBasedRouting: 5,
ingress: 12,
ingress: 14,
internalIngress: 1,
managedCertsForTLS: 2,
managedStaticGlobalIP: 1,
neg: 4,
pathBasedRouting: 4,
preSharedCertsForTLS: 4,
secretBasedCertsForTLS: 1,
staticGlobalIP: 1,
specifiedStaticGlobalIP: 1,
staticGlobalIP: 2,
tlsTermination: 5,
},
map[feature]int{
Expand Down

0 comments on commit dc867e5

Please sign in to comment.