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

[backend] CreatePVC's default storage class doesn't match default storage class set in the cluster #11396

Open
tumido opened this issue Nov 21, 2024 · 2 comments · May be fixed by #11417
Open

Comments

@tumido
Copy link
Contributor

tumido commented Nov 21, 2024

Kubernetes has a concept of a default Storage Class.

One would expect that by using kfp.kubernetes.CreatePVC without specifying storage_class_name results in using this default storage class. Instead, a hardcoded standard storage class is used (no matter if such class even exist on the cluster)

Environment

  • How did you deploy Kubeflow Pipelines (KFP)? - as part of Red Hat OpenShift AI
  • KFP version: 2.2.0
  • KFP SDK version: 2.10.1

Steps to reproduce

Use kfp.kubernetes.CreatePVC task without storage_class_name. A PVC with .spec.storageClassName: "standard" is created instead of one using the Storage class actually set as the default one in Kubernetes.

Expected result

Not specifying storage_class_name results in submitting the PVC without .spec.storageClassName property set and therefore result in the Kubernetes default storage class being applied.

Materials and Reference


Impacted by this bug? Give it a 👍.

@leseb
Copy link
Contributor

leseb commented Nov 27, 2024

👋🏻 I'm working on this.

leseb added a commit to leseb/pipelines that referenced this issue Nov 28, 2024
When using the SDK's function `CreatePVC`, leaving the
`storage_class_name` field empty will result in the cluster’s default
storage class being applied.

To enhance modularity and clarity, the logic for building the PVC
definition has been refactored into a dedicated function. Error messages
have been updated to align with this change, and unit tests have been
implemented to cover all required and optional fields.

In the code handling annotations, the `GetFields` method has replaced
the use of the `AsMap` method. This approach is more convenient and
eliminates the need for type conversion to `structpb.Value`.

Resolves: kubeflow#11396
Signed-off-by: Sébastien Han <[email protected]>
leseb added a commit to leseb/pipelines that referenced this issue Nov 28, 2024
When using the SDK's function `CreatePVC`, leaving the
`storage_class_name` field empty will result in the cluster’s default
storage class being applied.

To enhance modularity and clarity, the logic for building the PVC
definition has been refactored into a dedicated function. Error messages
have been updated to align with this change, and unit tests have been
implemented to cover all required and optional fields.

In the code handling annotations, the `GetFields` method has replaced
the use of the `AsMap` method. This approach is more convenient and
eliminates the need for type conversion to `structpb.Value`.

Resolves: kubeflow#11396
Signed-off-by: Sébastien Han <[email protected]>
leseb added a commit to leseb/pipelines that referenced this issue Nov 28, 2024
When using the SDK's function `CreatePVC`, leaving the
`storage_class_name` field empty will result in the cluster’s default
storage class being applied.

To enhance modularity and clarity, the logic for building the PVC
definition has been refactored into a dedicated function. Error messages
have been updated to align with this change, and unit tests have been
implemented to cover all required and optional fields.

In the code handling annotations, the `GetFields` method has replaced
the use of the `AsMap` method. This approach is more convenient and
eliminates the need for type conversion to `structpb.Value`.

Resolves: kubeflow#11396
Signed-off-by: Sébastien Han <[email protected]>
leseb added a commit to leseb/pipelines that referenced this issue Nov 28, 2024
When using the SDK's function `CreatePVC`, leaving the
`storage_class_name` field empty will result in the cluster’s default
storage class being applied.

To enhance modularity and clarity, the logic for building the PVC
definition has been refactored into a dedicated function. Error messages
have been updated to align with this change, and unit tests have been
implemented to cover all required and optional fields.

In the code handling annotations, the `GetFields` method has replaced
the use of the `AsMap` method. This approach is more convenient and
eliminates the need for type conversion to `structpb.Value`.

Resolves: kubeflow#11396
Signed-off-by: Sébastien Han <[email protected]>
@leseb
Copy link
Contributor

leseb commented Nov 28, 2024

#11417

leseb added a commit to leseb/pipelines that referenced this issue Nov 28, 2024
When using the SDK's function `CreatePVC`, leaving the
`storage_class_name` field empty will result in the cluster’s default
storage class being applied.

To enhance modularity and clarity, the logic for building the PVC
definition has been refactored into a dedicated function. Error messages
have been updated to align with this change, and unit tests have been
implemented to cover all required and optional fields.

In the code handling annotations, the `GetFields` method has replaced
the use of the `AsMap` method. This approach is more convenient and
eliminates the need for type conversion to `structpb.Value`.

Resolves: kubeflow#11396
Signed-off-by: Sébastien Han <[email protected]>
leseb added a commit to leseb/pipelines that referenced this issue Dec 9, 2024
When using the SDK's function `CreatePVC`, leaving the
`storage_class_name` field empty will result in the cluster’s default
storage class being applied.

To enhance modularity and clarity, the logic for building the PVC
definition has been refactored into a dedicated function. Error messages
have been updated to align with this change, and unit tests have been
implemented to cover all required and optional fields.

In the code handling annotations, the `GetFields` method has replaced
the use of the `AsMap` method. This approach is more convenient and
eliminates the need for type conversion to `structpb.Value`.

Resolves: kubeflow#11396
Signed-off-by: Sébastien Han <[email protected]>
@leseb leseb linked a pull request Dec 9, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants