Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting also field selectors for non-matching fields #1553

Closed
mdonkers opened this issue May 27, 2019 · 1 comment · Fixed by #1555
Closed

Allow setting also field selectors for non-matching fields #1553

mdonkers opened this issue May 27, 2019 · 1 comment · Fixed by #1555

Comments

@mdonkers
Copy link

The Kubernetes API spec mentions for field-selectors they support two operators;

Currently the Kubernetes client provides only the option to specify matching key-value pairs using withField(s). And with a hack its possible to do withField("key!", "value") which would result in key!=value (notice the exclamation mark after "key!").
But this might stop working when internals change.

Hence, to keep aligned with 'label' selectors, it would be nice to have a withFieldsNotMatching extension to the API for the Kubernetes client.

@mdonkers
Copy link
Author

(Started work on a PR to implement exactly this)

mdonkers pushed a commit to mdonkers/kubernetes-client that referenced this issue May 28, 2019
The Kubernetes API allows specifying field selectors where key-value
pair match, but also specifying non-matching key-value pairs using the
operator `!=`: https://kubernetes.io/docs/concepts/overview/working-with-objects/field-selectors/

Currently the Kubernetes Client allows specifying matching field
selectors using the `withField(key, value)` method. And by appending
a `!` (exclamation mark) after the key it's possible to 'hack' also the
non-matching pairs. But should internals change, this will quickly
break.

Instead this PR provides the necessary endpoint to specify non-matching
key-value pairs in field selectors by providing the method
`withFieldNot(key, value)`.

Some decisions to further discuss:

- Non-matching pairs can be specified multiple times with the same key.
Therefore a multi-value map is needed. To keep with the other filters
(labels), I used `Map<String, String[]>`. The alternative was to use
a list.
The downside of this decision is that a new String array needs
to be created when multiple values with the same key are added. But the
likeliness is quite small this will get called very often. And because
of the `merge` function on `Map`, for the first call it will directly
use the provided value, so no real overhead.

- The API is kept the same as `withField` however, and not like
`withLabelIn` which allows varargs. I figured this would be
non-intuitive. Downside here is that you cannot 'override' a key-value
pair. It would just add more values to the same key. But for me the
intuitiveness weighted stronger than being able to override pairs.
mdonkers pushed a commit to mdonkers/kubernetes-client that referenced this issue May 28, 2019
The Kubernetes API allows specifying field selectors where key-value
pair match, but also specifying non-matching key-value pairs using the
operator `!=`: https://kubernetes.io/docs/concepts/overview/working-with-objects/field-selectors/

Currently the Kubernetes Client allows specifying matching field
selectors using the `withField(key, value)` method. And by appending
a `!` (exclamation mark) after the key it's possible to 'hack' also the
non-matching pairs. But should internals change, this will quickly
break.

Instead this PR provides the necessary endpoint to specify non-matching
key-value pairs in field selectors by providing the method
`withFieldNot(key, value)`.

Some decisions to further discuss:

- Non-matching pairs can be specified multiple times with the same key.
Therefore a multi-value map is needed. To keep with the other filters
(labels), I used `Map<String, String[]>`. The alternative was to use
a list.
The downside of this decision is that a new String array needs
to be created when multiple values with the same key are added. But the
likeliness is quite small this will get called very often. And because
of the `merge` function on `Map`, for the first call it will directly
use the provided value, so no real overhead.

- The API is kept the same as `withField` however, and not like
`withLabelIn` which allows varargs. I figured this would be
non-intuitive. Downside here is that you cannot 'override' a key-value
pair. It would just add more values to the same key. But for me the
intuitiveness weighted stronger than being able to override pairs.
mdonkers pushed a commit to mdonkers/kubernetes-client that referenced this issue Jun 3, 2019
The Kubernetes API allows specifying field selectors where key-value
pair match, but also specifying non-matching key-value pairs using the
operator `!=`: https://kubernetes.io/docs/concepts/overview/working-with-objects/field-selectors/

Currently the Kubernetes Client allows specifying matching field
selectors using the `withField(key, value)` method. And by appending
a `!` (exclamation mark) after the key it's possible to 'hack' also the
non-matching pairs. But should internals change, this will quickly
break.

Instead this PR provides the necessary endpoint to specify non-matching
key-value pairs in field selectors by providing the method
`withFieldNot(key, value)`.

Some decisions to further discuss:

- Non-matching pairs can be specified multiple times with the same key.
Therefore a multi-value map is needed. To keep with the other filters
(labels), I used `Map<String, String[]>`. The alternative was to use
a list.
The downside of this decision is that a new String array needs
to be created when multiple values with the same key are added. But the
likeliness is quite small this will get called very often. And because
of the `merge` function on `Map`, for the first call it will directly
use the provided value, so no real overhead.

- The API is kept the same as `withField` however, and not like
`withLabelIn` which allows varargs. I figured this would be
non-intuitive. Downside here is that you cannot 'override' a key-value
pair. It would just add more values to the same key. But for me the
intuitiveness weighted stronger than being able to override pairs.
mdonkers pushed a commit to mdonkers/kubernetes-client that referenced this issue Jun 17, 2019
The Kubernetes API allows specifying field selectors where key-value
pair match, but also specifying non-matching key-value pairs using the
operator `!=`: https://kubernetes.io/docs/concepts/overview/working-with-objects/field-selectors/

Currently the Kubernetes Client allows specifying matching field
selectors using the `withField(key, value)` method. And by appending
a `!` (exclamation mark) after the key it's possible to 'hack' also the
non-matching pairs. But should internals change, this will quickly
break.

Instead this PR provides the necessary endpoint to specify non-matching
key-value pairs in field selectors by providing the method
`withFieldNot(key, value)`.

Some decisions to further discuss:

- Non-matching pairs can be specified multiple times with the same key.
Therefore a multi-value map is needed. To keep with the other filters
(labels), I used `Map<String, String[]>`. The alternative was to use
a list.
The downside of this decision is that a new String array needs
to be created when multiple values with the same key are added. But the
likeliness is quite small this will get called very often. And because
of the `merge` function on `Map`, for the first call it will directly
use the provided value, so no real overhead.

- The API is kept the same as `withField` however, and not like
`withLabelIn` which allows varargs. I figured this would be
non-intuitive. Downside here is that you cannot 'override' a key-value
pair. It would just add more values to the same key. But for me the
intuitiveness weighted stronger than being able to override pairs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant