Skip to content
This repository has been archived by the owner on Jan 3, 2023. It is now read-only.

Commit

Permalink
Merge pull request #139 from nikhiljindal/gcpProject
Browse files Browse the repository at this point in the history
Recreating HTTPS ingress should work without error
  • Loading branch information
nikhiljindal committed Mar 2, 2018
2 parents 252816a + 69aaf03 commit 9c91aa1
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 10 deletions.
4 changes: 3 additions & 1 deletion app/kubemci/pkg/gcp/sslcert/sslcertsyncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,10 @@ func sslCertMatches(desiredCert, existingCert compute.SslCertificate) bool {
existingCert.Id = 0
existingCert.SelfLink = ""

// Private key is write only, so we compare the certificate alone.
// We are assuming that no one will change just the key.
// NOTE: We do not print the diff, to not leak the certificate.
return reflect.DeepEqual(existingCert, desiredCert)
return reflect.DeepEqual(existingCert.Certificate, desiredCert.Certificate)
}

func (s *SSLCertSyncer) desiredSSLCert(lbName string, ing *v1beta1.Ingress, client kubeclient.Interface) (*compute.SslCertificate, error) {
Expand Down
10 changes: 8 additions & 2 deletions app/kubemci/pkg/ingress/ingresssyncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import (
)

var (
defaultIngressNamespace = "default"
defaultIngressNamespace = "default"
instanceGroupAnnotationKey = "ingress.gcp.kubernetes.io/instance-groups"
)

type IngressSyncer struct {
Expand Down Expand Up @@ -51,7 +52,12 @@ func (i *IngressSyncer) EnsureIngress(ing *v1beta1.Ingress, clients map[string]k
}
fmt.Println("Created Ingress in cluster:", cluster)
} else {
if !kubeutils.ObjectMetaAndSpecEquivalent(ing, existingIng) {
// Ignore instance group annotation while comparing ingresses.
// Its added by the in-cluster ingress-gce controller, so the existing ingress will have it while the user provided will not.
ignoreAnnotations := map[string]string{
instanceGroupAnnotationKey: "",
}
if !kubeutils.ObjectMetaAndSpecEquivalent(ing, existingIng, ignoreAnnotations) {
fmt.Printf("Found existing ingress resource which differs from the proposed one\n")
glog.V(2).Infof("Diff: %v\n", diff.ObjectDiff(ing, existingIng))
if forceUpdate {
Expand Down
24 changes: 20 additions & 4 deletions app/kubemci/pkg/kubeutils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,23 @@ func isSimpleHTTPProbe(probe *api_v1.Probe) bool {
(len(probe.Handler.HTTPGet.HTTPHeaders) == 1 && probe.Handler.HTTPGet.HTTPHeaders[0].Name == "Host")))
}

// Returns a copy of the given annotations map such that it does not contain keys that are present in the ignore map.
func ignoreAnnotations(annotations, ignore map[string]string) map[string]string {
result := map[string]string{}
for k, v := range annotations {
if _, exists := ignore[k]; !exists {
result[k] = v
}
}
return result
}

// Note: copied from https://github.com/kubernetes/federation/blob/7951a643cebc3abdcd903eaff90d1383b43928d1/pkg/federation-controller/util/meta.go#L61
// Checks if cluster-independent, user provided data in two given ObjectMeta are equal. If in
// the future the ObjectMeta structure is expanded then any field that is not populated
// by the api server should be included here.
func ObjectMetaEquivalent(a, b meta_v1.ObjectMeta) bool {
// Ignores annotations with keys in ignoreAnnotationKeys.
func ObjectMetaEquivalent(a, b meta_v1.ObjectMeta, ignoreAnnotationKeys map[string]string) bool {
if a.Name != b.Name {
return false
}
Expand All @@ -185,7 +197,10 @@ func ObjectMetaEquivalent(a, b meta_v1.ObjectMeta) bool {
if !reflect.DeepEqual(a.Labels, b.Labels) && (len(a.Labels) != 0 || len(b.Labels) != 0) {
return false
}
if !reflect.DeepEqual(a.Annotations, b.Annotations) && (len(a.Annotations) != 0 || len(b.Annotations) != 0) {
// Remove ignoreAnnotationKeys from the list of annotations.
aAnnotations := ignoreAnnotations(a.Annotations, ignoreAnnotationKeys)
bAnnotations := ignoreAnnotations(b.Annotations, ignoreAnnotationKeys)
if !reflect.DeepEqual(aAnnotations, bAnnotations) && (len(aAnnotations) != 0 || len(bAnnotations) != 0) {
return false
}
return true
Expand All @@ -194,10 +209,11 @@ func ObjectMetaEquivalent(a, b meta_v1.ObjectMeta) bool {
// Note: copied from https://github.com/kubernetes/federation/blob/7951a643cebc3abdcd903eaff90d1383b43928d1/pkg/federation-controller/util/meta.go#L79
// Checks if cluster-independent, user provided data in ObjectMeta and Spec in two given top
// level api objects are equivalent.
func ObjectMetaAndSpecEquivalent(a, b runtime.Object) bool {
// Ignores annotations with keys in ignoreAnnotationKeys.
func ObjectMetaAndSpecEquivalent(a, b runtime.Object, ignoreAnnotationKeys map[string]string) bool {
objectMetaA := reflect.ValueOf(a).Elem().FieldByName("ObjectMeta").Interface().(meta_v1.ObjectMeta)
objectMetaB := reflect.ValueOf(b).Elem().FieldByName("ObjectMeta").Interface().(meta_v1.ObjectMeta)
specA := reflect.ValueOf(a).Elem().FieldByName("Spec").Interface()
specB := reflect.ValueOf(b).Elem().FieldByName("Spec").Interface()
return ObjectMetaEquivalent(objectMetaA, objectMetaB) && reflect.DeepEqual(specA, specB)
return ObjectMetaEquivalent(objectMetaA, objectMetaB, ignoreAnnotationKeys) && reflect.DeepEqual(specA, specB)
}
4 changes: 1 addition & 3 deletions test/e2e/cases/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,7 @@ func testHTTPSIngress(project, kubeConfigPath, lbName string, kubectlArgs []stri
// Running create again should not return any error.
_, err = createIngress(project, kubeConfigPath, lbName, "examples/zone-printer/ingress/https-ingress.yaml")
if err != nil {
// TODO(nikhiljindal): Change this to unexpected fatal error once
// https://github.com/GoogleCloudPlatform/k8s-multicluster-ingress/issues/125 is fixed.
glog.Infof("Expected error in re-creating https ingress: %+v", err)
glog.Fatalf("Unexpected error in re-creating https ingress: %+v", err)
}

// TODO(nikhiljindal): Ensure that the ingress is created and deleted in all
Expand Down

0 comments on commit 9c91aa1

Please sign in to comment.