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

Marked specific fields as nullable to keep backwards compatibility #985

Conversation

jpkrohling
Copy link
Contributor

@jpkrohling jpkrohling commented Mar 24, 2020

When a CR like the following is applied, the current CRD validation will complain that it's invalid:

apiVersion: jaegertracing.io/v1
kind: Jaeger
metadata:
  name: simplest
spec:
  resources:
The Jaeger "simplest" is invalid: spec.resources: Invalid value: "null": spec.resources in body must be of type object: "null"

To keep backwards compatibility, we want to mark a few fields as nullable. With this PR, the YAML specified above is accepted.

Signed-off-by: Juraci Paixão Kröhling [email protected]

@jpkrohling jpkrohling requested a review from objectiser March 24, 2020 16:10
@codecov
Copy link

codecov bot commented Mar 24, 2020

Codecov Report

Merging #985 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #985   +/-   ##
=======================================
  Coverage   64.44%   64.44%           
=======================================
  Files          82       82           
  Lines        6526     6526           
=======================================
  Hits         4206     4206           
  Misses       2178     2178           
  Partials      142      142           
Impacted Files Coverage Δ
pkg/apis/jaegertracing/v1/jaeger_types.go 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc2cff4...d1eeea9. Read the comment docs.

@jpkrohling jpkrohling force-pushed the Backwards-compat-CRD-with-nullable-fields branch from 37ab874 to d1eeea9 Compare March 25, 2020 08:33
Copy link
Contributor

@objectiser objectiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, subject to CI passing and testing with maistra.

@kevinearls
Copy link
Contributor

LGTM. I was able to reproduce the initial problem, and then applying this PR fixed it.

@jpkrohling jpkrohling merged commit 3d27023 into jaegertracing:master Mar 25, 2020
@jpkrohling
Copy link
Contributor Author

Merging, but the verification that this is sufficient to address Maistra's problems is still pending.

@objectiser
Copy link
Contributor

@pavolloffay Could you update the CRD from this PR to the upstream operatorhub PRs for 1.17.1?

jpkrohling added a commit to jpkrohling/jaeger-operator that referenced this pull request Dec 15, 2020
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 this pull request may close these issues.

3 participants