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

Remove kubebuilder:pruning:PreserveUnknownFields annotation from CassandraDatacenterTemplate #653

Conversation

Miles-Garnsey
Copy link
Member

What this PR does:

Removes kubebuilder:pruning:PreserveUnknownFields, which allows CRs to skip validation and can lead to silent errors.

Which issue(s) this PR fixes:
#652

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CHANGELOG.md updated (not required for documentation PRs)
  • CLA Signed: DataStax CLA

@Miles-Garnsey Miles-Garnsey requested a review from a team as a code owner August 15, 2022 04:24
@Miles-Garnsey Miles-Garnsey marked this pull request as draft August 15, 2022 05:29
@Miles-Garnsey
Copy link
Member Author

Miles-Garnsey commented Aug 15, 2022

I made some changes which avoid an NPE here due to the config being nil. I'm still seeing a bunch of errors from the unit tests.

One of the failures is here, where we see that removing the annotation leads to quite a different output in terms of the final CassandraConfig object:

expected: &gabs.Container{object:map[string]interface {}{"cassandra-env-sh":map[string]interface {}{"additional-jvm-opts":[]interface {}{"-Dcassandra.system_distributed_replication=dc1:3,dc2:3", "-Dcom.sun.management.jmxremote.authenticate=true"}}, "cassandra-yaml":map[string]interface {}{"authenticator":"PasswordAuthenticator", "authorizer":"CassandraAuthorizer", "num_tokens":16, "role_manager":"CassandraRoleManager"}}}
actual  : &gabs.Container{object:map[string]interface {}{"cassandra-env-sh":map[string]interface {}{"additional-jvm-opts":[]interface {}{"-Dcassandra.system_distributed_replication=dc1:3,dc2:3", "-Dcom.sun.management.jmxremote.authenticate=true"}}, "cassandra-yaml":map[string]interface {}{"authenticator":"PasswordAuthenticator", "authorizer":"CassandraAuthorizer", "concurrent_reads":4, "concurrent_writes":4, "num_tokens":16, "role_manager":"CassandraRoleManager"}, "jvm-server-options":map[string]interface {}{"max_heap_size":1.073741824e+09}, "jvm11-server-options":map[string]interface {}{"garbage_collector":"G1GC"}}}

Complicating interpretation of the above output is that the actual and expected fields are swapped in the assertion. Once we take account of this, what appears to be happening is that the assertion is expecting far more fields to be defined than are actually being defined - presumably because they are "unknown fields".

@Miles-Garnsey
Copy link
Member Author

Fixed under #710

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.

None yet

1 participant