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

Fix a bug that would cause a crash if StorageClassName was left to nil #688

Merged
merged 4 commits into from
Aug 27, 2024

Conversation

burmanm
Copy link
Contributor

@burmanm burmanm commented Aug 22, 2024

What this PR does:

  • Changes handling of PersistentVolumeClaimSpec's *string in storageExpansion() method.

Which issue(s) this PR fixes:
Fixes #687

Checklist

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

@burmanm burmanm requested a review from a team as a code owner August 22, 2024 17:56
@garrynigel
Copy link

thanks @burmanm , Could we escalate and get this merged quickly?

if err != nil {
return result.Error(err)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

[question] Are we moving this inside the loop on the assumption that volume expansions will be rare?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes and this check is irrelevant in our other use cases right now.

@burmanm burmanm merged commit 18bf49d into k8ssandra:master Aug 27, 2024
42 checks passed
@garrynigel
Copy link

@burmanm thanks for merging this!! But I noticed in storage classes provided by cloud provider the default value is stored in annotations and not labels, so this always returns empty in my case.

See https://kubernetes.io/docs/tasks/administer-cluster/change-default-storage-class/#changing-the-default-storageclass

I modified the lines 166 to 175 slightly and only then it seems to be working.

	var defaultScName string
	var defaultScCount int
	
	if err := r.List(ctx, storageClassList); err != nil {
		return err
	}
	for i := range storageClassList.Items {
		storageClass := storageClassList.Items[i]
		if storageClass.Annotations != nil && storageClass.Annotations["storageclass.kubernetes.io/is-default-class"] == "true" {
			defaultScName = storageClass.Name
			defaultScCount++
		}
	
	if defaultScCount == 0 {
		return fmt.Errorf("no default storage class found, please specify StorageClassName in the CassandraDatacenter spec")
	} else if defaultScCount > 1 {
		return fmt.Errorf("found multiple default storage classes, please specify StorageClassName in the CassandraDatacenter spec")
	}

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.

cass-operator may crash if storageClassName is not set
3 participants