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.

Sign up for GitHub

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

Jump to bottom

Configure v3support #67

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Configure v3support #67

wants to merge 7 commits into from

Conversation

krpsh123
Copy link
Contributor

@krpsh123 krpsh123 commented Jan 16, 2025 •
edited
Loading

Description

  • Added an alert for the presence of an NFSStorageClass with parameters that are inconsistent with the settings in ModuleConfig.
  • Added parameter validation for NFSStorageClass and ModuleConfig in the controller and webhook.

Why do we need it, and what problem does it solve?

If the parameters of NFSStorageClass and ModuleConfig are inconsistent, PV with NFSv3 support will no longer mount on cluster nodes.

What is the expected result?

  • A user cannot create an NFSStorageClass with parameters inconsistent with the settings in ModuleConfig. If the webhook is disabled, the NFSStorageClass will still be created, but the corresponding StorageClass will not.
  • If the parameters in ModuleConfig are changed to values inconsistent with the parameters of existing NFSStorageClass instances, a label is added to the problematic StorageClass. Based on this label, Prometheus generates an alert: StorageClassDoesNotMatchTheSettingsInModuleConfigCsiNfs.

Checklist

  • The code is covered by unit tests.
  • e2e tests passed.
  • Documentation updated according to the changes.
  • Changes were tested in the Kubernetes cluster manually.

@krpsh123 krpsh123 added the enhancement New feature or request label Jan 16, 2025
@krpsh123 krpsh123 self-assigned this Jan 18, 2025
@krpsh123 krpsh123 marked this pull request as ready for review January 20, 2025 06:58
@krpsh123
resolve merge conflict
4582a8f 
Signed-off-by: Pavel Karpov <[email protected]>
duckhawk
duckhawk reviewed Jan 22, 2025
View reviewed changes
templates/webhooks/webhook.yaml
rules:
- apiGroups: ["deckhouse.io"]
apiVersions: ["v1alpha1"]
operations: ["UPDATE"]
Copy link
Member

Choose a reason for hiding this comment

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

Why we ignore CREATE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When creating a ModuleConfig, there should be no NFSStorageClass in the cluster, so no CREATE is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@duckhawk duckhawk duckhawk left review comments

@AleksZimin AleksZimin Awaiting requested review from AleksZimin

Assignees

@krpsh123 krpsh123

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@krpsh123 @duckhawk