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

Safely set annotation on datacenter #254

Merged
merged 3 commits into from
Jan 17, 2022

Conversation

akshaymankar
Copy link
Contributor

@akshaymankar akshaymankar commented Jan 11, 2022

What this PR does:

While setting any annotation, the user must initialize the Annotations
map if it isn't already. The helper metav1.SetMetadataAnnotation takes
care of this.

Which issue(s) this PR fixes:
N/A
The updateConfigHashAnnotation function panics when it encounters a datacenter without any annotations. This PR should fix that.

Checklist

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

While setting any annotation, the user must initialize the Annotations
map if it isn't already. The helper `metav1.SetMetadataAnnotation` takes
care of this.
@akshaymankar akshaymankar requested a review from a team as a code owner January 11, 2022 10:35
@burmanm
Copy link
Contributor

burmanm commented Jan 11, 2022

Hi, thank you for the PR. Have you run into a situation where this actually occurs?

@akshaymankar
Copy link
Contributor Author

Hi, thank you for the PR. Have you run into a situation where this actually occurs?

Yes, I have. The issue only occurs when using a separate kubernetes secret for the cassandra config overrides. The cass-operator gets stuck into a crash loop with this error at the end of the logs:

panic: assignment to entry in nil map

goroutine 328 [running]:
github.com/k8ssandra/cass-operator/pkg/reconciliation.(*ReconciliationContext).updateConfigHashAnnotation(0xc0008c4000, 0xc0008c52c0, 0x18a04b4, 0x6)
        /workspace/pkg/reconciliation/reconcile_configsecret.go:108 +0x1f3
github.com/k8ssandra/cass-operator/pkg/reconciliation.(*ReconciliationContext).CheckConfigSecret(0xc0008c4000, 0x1a95300, 0x2505678)
        /workspace/pkg/reconciliation/reconcile_configsecret.go:58 +0x5e6
github.com/k8ssandra/cass-operator/pkg/reconciliation.(*ReconciliationContext).ReconcileAllRacks(0xc0008c4000, 0x0, 0x0, 0x0, 0x0)
        /workspace/pkg/reconciliation/reconcile_racks.go:2435 +0x50b
github.com/k8ssandra/cass-operator/pkg/reconciliation.(*ReconciliationContext).CalculateReconciliationActions(0xc0008c4000, 0x0, 0x0, 0xc06f72bab7b14d8c, 0x45b80d114)
        /workspace/pkg/reconciliation/handler.go:160 +0x1b7
github.com/k8ssandra/cass-operator/controllers/cassandra.(*CassandraDatacenterReconciler).Reconcile(0xc000343e00, 0x1abb878, 0xc0007ea480, 0xc000253e34, 0x7, 0xc00013fd58, 0x15, 0xc0007ea400, 0x0, 0x0, ...)
        /workspace/controllers/cassandra/cassandradatacenter_controller.go:140 +0x8c7
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc000531a40, 0x1abb7d0, 0xc0004d9d00, 0x172dea0, 0xc000638140)
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.2/pkg/internal/controller/controller.go:298 +0x30d
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc000531a40, 0x1abb7d0, 0xc0004d9d00, 0x4d82f00)
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.2/pkg/internal/controller/controller.go:253 +0x205
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2(0xc0000495c0, 0xc000531a40, 0x1abb7d0, 0xc0004d9d00)
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.2/pkg/internal/controller/controller.go:214 +0x6b
created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.2/pkg/internal/controller/controller.go:210 +0x425

A workaround for this is to set any annotation on the datacenter object.

@burmanm
Copy link
Contributor

burmanm commented Jan 11, 2022

Apparently it's a bit different configuration than the one used in https://github.com/k8ssandra/cass-operator/blob/master/tests/config_secret/config_secret_suite_test.go ?

Do you think it would be possible to modify that integration test to also trigger this error? To ensure it's not unintentionally reintroduced (by botched mergeconflict for example).

@burmanm burmanm merged commit 4bb13a0 into k8ssandra:master Jan 17, 2022
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.

2 participants