Don't try to write the wrong UID, version on Federated Ingress updates.

Fixes #33135.
This commit is contained in:
Quinton Hoole 2016-09-22 15:11:55 -07:00
parent cf8fcd03f0
commit 359bd17066
3 changed files with 69 additions and 36 deletions

View File

@ -624,10 +624,7 @@ func (ic *IngressController) reconcileIngress(ingress types.NamespacedName) {
ic.deliverIngress(ingress, 0, true)
return
}
desiredIngress := &extensions_v1beta1.Ingress{
ObjectMeta: baseIngress.ObjectMeta,
Spec: baseIngress.Spec,
}
desiredIngress := &extensions_v1beta1.Ingress{}
objMeta, err := conversion.NewCloner().DeepCopy(baseIngress.ObjectMeta)
if err != nil {
glog.Errorf("Error deep copying ObjectMeta: %v", err)
@ -649,8 +646,7 @@ func (ic *IngressController) reconcileIngress(ingress types.NamespacedName) {
if !clusterIngressFound {
glog.V(4).Infof("No existing Ingress %s in cluster %s - checking if appropriate to queue a create operation", ingress, cluster.Name)
// We can't supply server-created fields when creating a new object.
desiredIngress.ObjectMeta.ResourceVersion = ""
desiredIngress.ObjectMeta.UID = ""
desiredIngress.ObjectMeta = util.DeepCopyObjectMeta(baseIngress.ObjectMeta)
ic.eventRecorder.Eventf(baseIngress, api.EventTypeNormal, "CreateInCluster",
"Creating ingress in cluster %s", cluster.Name)
@ -707,29 +703,43 @@ func (ic *IngressController) reconcileIngress(ingress types.NamespacedName) {
glog.V(4).Infof(logStr, "Not transferring")
}
// Update existing cluster ingress, if needed.
if util.ObjectMetaEquivalent(desiredIngress.ObjectMeta, clusterIngress.ObjectMeta) &&
reflect.DeepEqual(desiredIngress.Spec, clusterIngress.Spec) &&
reflect.DeepEqual(baseIngress.Status.LoadBalancer.Ingress, clusterIngress.Status.LoadBalancer.Ingress) {
if util.ObjectMetaEquivalent(baseIngress.ObjectMeta, clusterIngress.ObjectMeta) &&
reflect.DeepEqual(baseIngress.Spec, clusterIngress.Spec) {
glog.V(4).Infof("Ingress %q in cluster %q does not need an update: cluster ingress is equivalent to federated ingress", ingress, cluster.Name)
} else {
glog.V(4).Infof("Ingress %s in cluster %s needs an update: cluster ingress %v is not equivalent to federated ingress %v", ingress, cluster.Name, clusterIngress, desiredIngress)
if !util.ObjectMetaEquivalent(desiredIngress.ObjectMeta, clusterIngress.ObjectMeta) {
// Merge any annotations on the federated ingress onto the underlying cluster ingress,
// overwriting duplicates.
for key, val := range baseIngress.ObjectMeta.Annotations {
desiredIngress.ObjectMeta.Annotations[key] = val
}
ic.eventRecorder.Eventf(baseIngress, api.EventTypeNormal, "UpdateInCluster",
"Updating ingress in cluster %s", cluster.Name)
operations = append(operations, util.FederatedOperation{
Type: util.OperationTypeUpdate,
Obj: desiredIngress,
ClusterName: cluster.Name,
})
// TODO: Transfer any readonly (target-proxy, url-map etc) annotations from the master cluster to the federation, if this is the master cluster.
// This is only for consistency, so that the federation ingress metadata matches the underlying clusters. It's not actually required.
objMeta, err := conversion.NewCloner().DeepCopy(clusterIngress.ObjectMeta)
if err != nil {
glog.Errorf("Error deep copying ObjectMeta: %v", err)
}
desiredIngress.ObjectMeta, ok = objMeta.(v1.ObjectMeta)
if !ok {
glog.Errorf("Internal error: Failed to cast to v1.ObjectMeta: %v", objMeta)
}
// Merge any annotations and labels on the federated ingress onto the underlying cluster ingress,
// overwriting duplicates.
if desiredIngress.ObjectMeta.Annotations == nil {
desiredIngress.ObjectMeta.Annotations = make(map[string]string)
}
for key, val := range baseIngress.ObjectMeta.Annotations {
desiredIngress.ObjectMeta.Annotations[key] = val
}
if desiredIngress.ObjectMeta.Labels == nil {
desiredIngress.ObjectMeta.Labels = make(map[string]string)
}
for key, val := range baseIngress.ObjectMeta.Labels {
desiredIngress.ObjectMeta.Labels[key] = val
}
ic.eventRecorder.Eventf(baseIngress, api.EventTypeNormal, "UpdateInCluster",
"Updating ingress in cluster %s", cluster.Name)
operations = append(operations, util.FederatedOperation{
Type: util.OperationTypeUpdate,
Obj: desiredIngress,
ClusterName: cluster.Name,
})
// TODO: Transfer any readonly (target-proxy, url-map etc) annotations from the master cluster to the federation, if this is the master cluster.
// This is only for consistency, so that the federation ingress metadata matches the underlying clusters. It's not actually required }
}
}
}

View File

@ -96,10 +96,10 @@ func TestIngressController(t *testing.T) {
ing1 := extensions_v1beta1.Ingress{
ObjectMeta: api_v1.ObjectMeta{
Name: "test-ingress",
Namespace: "mynamespace",
SelfLink: "/api/v1/namespaces/mynamespace/ingress/test-ingress",
Annotations: map[string]string{},
Name: "test-ingress",
Namespace: "mynamespace",
SelfLink: "/api/v1/namespaces/mynamespace/ingress/test-ingress",
// TODO: Remove: Annotations: map[string]string{},
},
Status: extensions_v1beta1.IngressStatus{
LoadBalancer: api_v1.LoadBalancerStatus{
@ -139,6 +139,9 @@ func TestIngressController(t *testing.T) {
}
// Test update federated ingress.
if updatedIngress.ObjectMeta.Annotations == nil {
updatedIngress.ObjectMeta.Annotations = make(map[string]string)
}
updatedIngress.ObjectMeta.Annotations["A"] = "B"
t.Log("Modifying Federated Ingress")
fedIngressWatch.Modify(updatedIngress)
@ -146,7 +149,7 @@ func TestIngressController(t *testing.T) {
updatedIngress2 := GetIngressFromChan(t, cluster1IngressUpdateChan)
assert.NotNil(t, updatedIngress2)
assert.True(t, reflect.DeepEqual(updatedIngress2.Spec, updatedIngress.Spec), "Spec of updated ingress is not equal")
assert.True(t, util.ObjectMetaEquivalent(updatedIngress2.ObjectMeta, updatedIngress.ObjectMeta), "Metadata of updated object is not equivalent")
assert.Equal(t, updatedIngress2.ObjectMeta.Annotations["A"], updatedIngress.ObjectMeta.Annotations["A"], "Updated annotation not transferred from federated to cluster ingress.")
// Test add cluster
t.Log("Adding a second cluster")
ing1.Annotations[staticIPNameKeyWritable] = "foo" // Make sure that the base object has a static IP name first.
@ -194,10 +197,10 @@ func GetClusterFromChan(c chan runtime.Object) *federation_api.Cluster {
func NewConfigMap(uid string) *api_v1.ConfigMap {
return &api_v1.ConfigMap{
ObjectMeta: api_v1.ObjectMeta{
Name: uidConfigMapName,
Namespace: uidConfigMapNamespace,
SelfLink: "/api/v1/namespaces/" + uidConfigMapNamespace + "/configmap/" + uidConfigMapName,
Annotations: map[string]string{},
Name: uidConfigMapName,
Namespace: uidConfigMapNamespace,
SelfLink: "/api/v1/namespaces/" + uidConfigMapNamespace + "/configmap/" + uidConfigMapName,
// TODO: Remove: Annotations: map[string]string{},
},
Data: map[string]string{
uidKey: uid,

View File

@ -23,7 +23,7 @@ import (
)
// Copies cluster-independent, user provided data from the given ObjectMeta struct. If in
// the future the ObjectMeta structure is expanded then any field that is not populared
// the future the ObjectMeta structure is expanded then any field that is not populated
// by the api server should be included here.
func CopyObjectMeta(obj api_v1.ObjectMeta) api_v1.ObjectMeta {
return api_v1.ObjectMeta{
@ -34,8 +34,28 @@ func CopyObjectMeta(obj api_v1.ObjectMeta) api_v1.ObjectMeta {
}
}
// Deep copies cluster-independent, user provided data from the given ObjectMeta struct. 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 DeepCopyObjectMeta(obj api_v1.ObjectMeta) api_v1.ObjectMeta {
copyMeta := CopyObjectMeta(obj)
if obj.Labels != nil {
copyMeta.Labels = make(map[string]string)
for key, val := range obj.Labels {
copyMeta.Labels[key] = val
}
}
if obj.Annotations != nil {
copyMeta.Annotations = make(map[string]string)
for key, val := range obj.Annotations {
copyMeta.Annotations[key] = val
}
}
return copyMeta
}
// Checks if cluster-independed, user provided data in two given ObjectMeta are eqaul. If in
// the future the ObjectMeta structure is expanded then any field that is not populared
// 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 api_v1.ObjectMeta) bool {
if a.Name != b.Name {