Skip to content

Commit

Permalink
Avoid clearing objects in conversion funcs
Browse files Browse the repository at this point in the history
This removes the behavior of mutating the objects received from the
client-go library. To begin with there isn't really any benefit from
doing so, given we don't store the object afterwards, and it will be
ready for gc when it leaves the scope inside client-go. client-go can
possibly return the same pointer twice here, to trigger eg. both an
object update delta and then a DeletedFinalStateUnknown delta with the
same pointer.

For more info, see the issue 115658 in the kubernetes/kubernetes repo on
github.

Signed-off-by: Odin Ugedal <ougedal@palantir.com>
Signed-off-by: Odin Ugedal <odin@uged.al>
  • Loading branch information
odinuge authored and borkmann committed Mar 13, 2023
1 parent 2cdd4ee commit 74307f1
Showing 1 changed file with 0 additions and 12 deletions.
12 changes: 0 additions & 12 deletions pkg/k8s/factory_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,8 +559,6 @@ func ConvertToK8sService(obj interface{}) interface{} {
// *types.SlimCNP, also without the Status field of the given CNP, in its Obj.
// If the given obj can't be cast into either *cilium_v2.CiliumClusterwideNetworkPolicy
// nor cache.DeletedFinalStateUnknown, the original obj is returned.
// WARNING calling this function will set *all* fields of the given CNP as
// empty.
func ConvertToCCNP(obj interface{}) interface{} {
switch concreteObj := obj.(type) {
case *cilium_v2.CiliumClusterwideNetworkPolicy:
Expand All @@ -572,7 +570,6 @@ func ConvertToCCNP(obj interface{}) interface{} {
Specs: concreteObj.Specs,
},
}
*concreteObj = cilium_v2.CiliumClusterwideNetworkPolicy{}
return ccnp

case cache.DeletedFinalStateUnknown:
Expand All @@ -592,7 +589,6 @@ func ConvertToCCNP(obj interface{}) interface{} {
Key: concreteObj.Key,
Obj: slimCNP,
}
*ccnp = cilium_v2.CiliumClusterwideNetworkPolicy{}
return dfsu

default:
Expand All @@ -606,8 +602,6 @@ func ConvertToCCNP(obj interface{}) interface{} {
// *types.SlimCNP, also without the Status field of the given CNP, in its Obj.
// If the given obj can't be cast into either *cilium_v2.CiliumNetworkPolicy
// nor cache.DeletedFinalStateUnknown, the original obj is returned.
// WARNING calling this function will set *all* fields of the given CNP as
// empty.
func ConvertToCNP(obj interface{}) interface{} {
switch concreteObj := obj.(type) {
case *cilium_v2.CiliumNetworkPolicy:
Expand All @@ -619,7 +613,6 @@ func ConvertToCNP(obj interface{}) interface{} {
Specs: concreteObj.Specs,
},
}
*concreteObj = cilium_v2.CiliumNetworkPolicy{}
return cnp
case cache.DeletedFinalStateUnknown:
cnp, ok := concreteObj.Obj.(*cilium_v2.CiliumNetworkPolicy)
Expand All @@ -637,7 +630,6 @@ func ConvertToCNP(obj interface{}) interface{} {
},
},
}
*cnp = cilium_v2.CiliumNetworkPolicy{}
return dfsu
default:
return obj
Expand Down Expand Up @@ -692,8 +684,6 @@ func convertToTaints(v1Taints []v1.Taint) []slim_corev1.Taint {
// a cache.DeletedFinalStateUnknown with a *types.Node in its Obj.
// If the given obj can't be cast into either *v1.Node
// nor cache.DeletedFinalStateUnknown, the original obj is returned.
// WARNING calling this function will set *all* fields of the given Node as
// empty.
func ConvertToNode(obj interface{}) interface{} {
switch concreteObj := obj.(type) {
case *v1.Node:
Expand All @@ -719,7 +709,6 @@ func ConvertToNode(obj interface{}) interface{} {
Addresses: convertToAddress(concreteObj.Status.Addresses),
},
}
*concreteObj = v1.Node{}
return p
case cache.DeletedFinalStateUnknown:
node, ok := concreteObj.Obj.(*v1.Node)
Expand Down Expand Up @@ -751,7 +740,6 @@ func ConvertToNode(obj interface{}) interface{} {
},
},
}
*node = v1.Node{}
return dfsu
default:
return obj
Expand Down

0 comments on commit 74307f1

Please sign in to comment.