Arnavion / k8s-openapi

Rust definitions of the resource types in the Kubernetes client API
Apache License 2.0
373 stars 42 forks source link

Comparison of elements is too strict #151

Closed anagno closed 8 months ago

anagno commented 8 months ago

Hello and thank you for the great project !

I have a use case that I want to compare some deployments and see if they are equal. Let`s take for example the below 2 deployments:

apiVersion: apps/v1
kind: Deployment
metadata:
    name: test
spec:
    replicas: 1
    template:
        spec:
            containers:
                - env:
                    - name: test
                      valueFrom:
                        configMapKeyRef:
                            name: test
                            key: test
                    - name: test2
                      valueFrom:
                        configMapKeyRef:
                            name: test2
                            key: test2
                  name: test
                  image: test:2.0.0
                  imagePullPolicy: IfNotPresent
apiVersion: apps/v1
kind: Deployment
metadata:
    name: test
spec:
    replicas: 1
    template:
        spec:
            containers:
                - env:
                    - name: test2
                      valueFrom:
                        configMapKeyRef:
                            name: test2
                            key: test2
                    - name: test
                      valueFrom:
                        configMapKeyRef:
                            name: test
                            key: test
                  name: test
                  image: test:2.0.0
                  imagePullPolicy: IfNotPresent

The only difference of these deployments is the order of the environmental variables.

When we compare them using the crate, the comparison returns false. Is that expected? Is there a way of comparing and ignoring the order of the elements?

I am guessing this would also happen for other elements and Kinds

KR,

Vasileios

Arnavion commented 8 months ago

When we compare them using the crate, the comparison returns false. Is that expected?

Well, it's a Vec because the spec says it's an array. It does have "x-kubernetes-patch-strategy": "merge", "x-kubernetes-patch-merge-key": "name" so it functions like a map keyed by .name when merged, but the representation is an array regardless.

This should be the behavior of every client library. Eg both Python and C# also represent V1Container::env as an array. With Python:

import kubernetes

c1 = kubernetes.client.V1Container(name='foo', env=[kubernetes.client.V1EnvVar('a', 'x'), kubernetes.client.V1EnvVar('b', 'y')])
c2 = kubernetes.client.V1Container(name='foo', env=[kubernetes.client.V1EnvVar('b', 'y'), kubernetes.client.V1EnvVar('a', 'x')])
print(c1 == c2) # False

c1.env = sorted(c1.env, key=lambda e: e.name)
c2.env = sorted(c2.env, key=lambda e: e.name)
print(c1 == c2) # True

Is there a way of comparing and ignoring the order of the elements?

We could have a custom PartialEq impl for "x-kubernetes-patch-strategy": "merge" fields to do a sorted comparison, but that would not be in the spirit of PartialEq.

It would be better for you who owns the Container to sort its .env if you intend to compare it for equality without caring about order.

anagno commented 8 months ago

It would be better for you who owns the Container to sort its .env if you intend to compare it for equality without caring about order.

Thanks for the info ! Makes sense. I will just sort the environmental variables then!