Commit ab184e1b authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #36399 from mwielgus/fedrc1

Automatic merge from submit-queue Make desired objects in fed controller reconcilation function fully writable When creating desired objects for sub-cluster in reconciliate functions we don't make full copies. This may result in unexpected race condition in the future when someone actually adds some object modifications (not needed in most cases but anyway - the cost of extra copy will be relatively small comparing to inter-cluster traffic). In case of ReplicaSet and Deployment the spec copy was a shallow one - ok for setting just replicas but also error-prone in the future. cc: @quinton-hoole @nikhiljindal @madhusudancs
parents 0042ce56 f91b3fee
...@@ -269,8 +269,9 @@ func (configmapcontroller *ConfigMapController) reconcileConfigMap(configmap typ ...@@ -269,8 +269,9 @@ func (configmapcontroller *ConfigMapController) reconcileConfigMap(configmap typ
return return
} }
// Do not modify data.
desiredConfigMap := &api_v1.ConfigMap{ desiredConfigMap := &api_v1.ConfigMap{
ObjectMeta: util.CopyObjectMeta(baseConfigMap.ObjectMeta), ObjectMeta: util.DeepCopyRelevantObjectMeta(baseConfigMap.ObjectMeta),
Data: baseConfigMap.Data, Data: baseConfigMap.Data,
} }
......
...@@ -401,9 +401,10 @@ func (daemonsetcontroller *DaemonSetController) reconcileDaemonSet(namespace str ...@@ -401,9 +401,10 @@ func (daemonsetcontroller *DaemonSetController) reconcileDaemonSet(namespace str
return return
} }
// Do not modify. Otherwise make a deep copy.
desiredDaemonSet := &extensionsv1.DaemonSet{ desiredDaemonSet := &extensionsv1.DaemonSet{
ObjectMeta: util.CopyObjectMeta(baseDaemonSet.ObjectMeta), ObjectMeta: util.DeepCopyRelevantObjectMeta(baseDaemonSet.ObjectMeta),
Spec: baseDaemonSet.Spec, Spec: util.DeepCopyApiTypeOrPanic(baseDaemonSet.Spec).(extensionsv1.DaemonSetSpec),
} }
if !found { if !found {
......
...@@ -561,9 +561,10 @@ func (fdc *DeploymentController) reconcileDeployment(key string) (reconciliation ...@@ -561,9 +561,10 @@ func (fdc *DeploymentController) reconcileDeployment(key string) (reconciliation
return statusError, err return statusError, err
} }
// The object can be modified.
ld := &extensionsv1.Deployment{ ld := &extensionsv1.Deployment{
ObjectMeta: fedutil.CopyObjectMeta(fd.ObjectMeta), ObjectMeta: fedutil.DeepCopyRelevantObjectMeta(fd.ObjectMeta),
Spec: fd.Spec, Spec: fedutil.DeepCopyApiTypeOrPanic(fd.Spec).(extensionsv1.DeploymentSpec),
} }
specReplicas := int32(replicas) specReplicas := int32(replicas)
ld.Spec.Replicas = &specReplicas ld.Spec.Replicas = &specReplicas
......
...@@ -735,7 +735,7 @@ func (ic *IngressController) reconcileIngress(ingress types.NamespacedName) { ...@@ -735,7 +735,7 @@ func (ic *IngressController) reconcileIngress(ingress types.NamespacedName) {
if !clusterIngressFound { if !clusterIngressFound {
glog.V(4).Infof("No existing Ingress %s in cluster %s - checking if appropriate to queue a create operation", ingress, cluster.Name) 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. // We can't supply server-created fields when creating a new object.
desiredIngress.ObjectMeta = util.DeepCopyObjectMeta(baseIngress.ObjectMeta) desiredIngress.ObjectMeta = util.DeepCopyRelevantObjectMeta(baseIngress.ObjectMeta)
ic.eventRecorder.Eventf(baseIngress, api.EventTypeNormal, "CreateInCluster", ic.eventRecorder.Eventf(baseIngress, api.EventTypeNormal, "CreateInCluster",
"Creating ingress in cluster %s", cluster.Name) "Creating ingress in cluster %s", cluster.Name)
......
...@@ -391,9 +391,10 @@ func (nc *NamespaceController) reconcileNamespace(namespace string) { ...@@ -391,9 +391,10 @@ func (nc *NamespaceController) reconcileNamespace(namespace string) {
nc.deliverNamespace(namespace, 0, true) nc.deliverNamespace(namespace, 0, true)
return return
} }
// The object should not be modified.
desiredNamespace := &api_v1.Namespace{ desiredNamespace := &api_v1.Namespace{
ObjectMeta: util.CopyObjectMeta(baseNamespace.ObjectMeta), ObjectMeta: util.DeepCopyRelevantObjectMeta(baseNamespace.ObjectMeta),
Spec: baseNamespace.Spec, Spec: util.DeepCopyApiTypeOrPanic(baseNamespace.Spec).(api_v1.NamespaceSpec),
} }
glog.V(5).Infof("Desired namespace in underlying clusters: %+v", desiredNamespace) glog.V(5).Infof("Desired namespace in underlying clusters: %+v", desiredNamespace)
......
...@@ -562,9 +562,10 @@ func (frsc *ReplicaSetController) reconcileReplicaSet(key string) (reconciliatio ...@@ -562,9 +562,10 @@ func (frsc *ReplicaSetController) reconcileReplicaSet(key string) (reconciliatio
return statusError, err return statusError, err
} }
// The object can be modified.
lrs := &extensionsv1.ReplicaSet{ lrs := &extensionsv1.ReplicaSet{
ObjectMeta: fedutil.CopyObjectMeta(frs.ObjectMeta), ObjectMeta: fedutil.DeepCopyRelevantObjectMeta(frs.ObjectMeta),
Spec: frs.Spec, Spec: fedutil.DeepCopyApiTypeOrPanic(frs.Spec).(extensionsv1.ReplicaSetSpec),
} }
specReplicas := int32(replicas) specReplicas := int32(replicas)
lrs.Spec.Replicas = &specReplicas lrs.Spec.Replicas = &specReplicas
......
...@@ -366,8 +366,9 @@ func (secretcontroller *SecretController) reconcileSecret(secret types.Namespace ...@@ -366,8 +366,9 @@ func (secretcontroller *SecretController) reconcileSecret(secret types.Namespace
return return
} }
// The data should not be modified.
desiredSecret := &api_v1.Secret{ desiredSecret := &api_v1.Secret{
ObjectMeta: util.CopyObjectMeta(baseSecret.ObjectMeta), ObjectMeta: util.DeepCopyRelevantObjectMeta(baseSecret.ObjectMeta),
Data: baseSecret.Data, Data: baseSecret.Data,
Type: baseSecret.Type, Type: baseSecret.Type,
} }
......
...@@ -36,6 +36,7 @@ go_library( ...@@ -36,6 +36,7 @@ go_library(
"//pkg/client/restclient:go_default_library", "//pkg/client/restclient:go_default_library",
"//pkg/client/unversioned/clientcmd:go_default_library", "//pkg/client/unversioned/clientcmd:go_default_library",
"//pkg/client/unversioned/clientcmd/api:go_default_library", "//pkg/client/unversioned/clientcmd/api:go_default_library",
"//pkg/conversion:go_default_library",
"//pkg/runtime:go_default_library", "//pkg/runtime:go_default_library",
"//pkg/util/flowcontrol:go_default_library", "//pkg/util/flowcontrol:go_default_library",
"//pkg/util/net:go_default_library", "//pkg/util/net:go_default_library",
......
...@@ -20,13 +20,14 @@ import ( ...@@ -20,13 +20,14 @@ import (
"reflect" "reflect"
api_v1 "k8s.io/kubernetes/pkg/api/v1" api_v1 "k8s.io/kubernetes/pkg/api/v1"
"k8s.io/kubernetes/pkg/conversion"
"k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/runtime"
) )
// Copies cluster-independent, user provided data from the given ObjectMeta struct. If in // 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 // the future the ObjectMeta structure is expanded then any field that is not populated
// by the api server should be included here. // by the api server should be included here.
func CopyObjectMeta(obj api_v1.ObjectMeta) api_v1.ObjectMeta { func copyObjectMeta(obj api_v1.ObjectMeta) api_v1.ObjectMeta {
return api_v1.ObjectMeta{ return api_v1.ObjectMeta{
Name: obj.Name, Name: obj.Name,
Namespace: obj.Namespace, Namespace: obj.Namespace,
...@@ -38,8 +39,8 @@ func CopyObjectMeta(obj api_v1.ObjectMeta) api_v1.ObjectMeta { ...@@ -38,8 +39,8 @@ func CopyObjectMeta(obj api_v1.ObjectMeta) api_v1.ObjectMeta {
// Deep copies cluster-independent, user provided data from the given ObjectMeta struct. If in // 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 // the future the ObjectMeta structure is expanded then any field that is not populated
// by the api server should be included here. // by the api server should be included here.
func DeepCopyObjectMeta(obj api_v1.ObjectMeta) api_v1.ObjectMeta { func DeepCopyRelevantObjectMeta(obj api_v1.ObjectMeta) api_v1.ObjectMeta {
copyMeta := CopyObjectMeta(obj) copyMeta := copyObjectMeta(obj)
if obj.Labels != nil { if obj.Labels != nil {
copyMeta.Labels = make(map[string]string) copyMeta.Labels = make(map[string]string)
for key, val := range obj.Labels { for key, val := range obj.Labels {
...@@ -83,3 +84,11 @@ func ObjectMetaAndSpecEquivalent(a, b runtime.Object) bool { ...@@ -83,3 +84,11 @@ func ObjectMetaAndSpecEquivalent(a, b runtime.Object) bool {
specB := reflect.ValueOf(b).Elem().FieldByName("Spec").Interface() specB := reflect.ValueOf(b).Elem().FieldByName("Spec").Interface()
return ObjectMetaEquivalent(objectMetaA, objectMetaB) && reflect.DeepEqual(specA, specB) return ObjectMetaEquivalent(objectMetaA, objectMetaB) && reflect.DeepEqual(specA, specB)
} }
func DeepCopyApiTypeOrPanic(item interface{}) interface{} {
result, err := conversion.NewCloner().DeepCopy(item)
if err != nil {
panic(err)
}
return result
}
...@@ -31,7 +31,7 @@ func TestObjectMeta(t *testing.T) { ...@@ -31,7 +31,7 @@ func TestObjectMeta(t *testing.T) {
UID: "1231231412", UID: "1231231412",
ResourceVersion: "999", ResourceVersion: "999",
} }
o2 := CopyObjectMeta(o1) o2 := copyObjectMeta(o1)
o3 := api_v1.ObjectMeta{ o3 := api_v1.ObjectMeta{
Namespace: "ns1", Namespace: "ns1",
Name: "s1", Name: "s1",
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment