Fix reference dataset pv name to avoid Released PV re-used

[kebe7jun]

No description provided.

[gemini-code-assist]

Summary of Changes

Hello @kebe7jun, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request primarily addresses a critical issue related to Persistent Volume (PV) naming within the dataset controller. By incorporating a unique identifier into the PV naming scheme, the changes prevent the unintended reuse of PVs, which could lead to data integrity issues or conflicts. Additionally, the PR includes refactoring to enhance code clarity through the use of constants and introduces an optimization to streamline the PVC reconciliation process, improving overall controller efficiency.

Highlights

• Unique Persistent Volume Naming: The naming convention for newly created Persistent Volumes (PVs) associated with datasets has been modified. The new names now incorporate a portion of the Dataset's Unique ID (UID), ensuring that each dataset's PV has a globally unique name. This change is crucial for preventing potential conflicts and unintended reuse of PVs, especially in scenarios where datasets might be released or deleted and their associated PVs could otherwise be re-provisioned with the same name.
• Refactoring of Condition Type Constants: Explicit constants have been introduced for various condition types (e.g., PVC, JobStatus, Job, ConfigMap) within the dataset controller. These constants are now consistently used throughout the reconciliation logic, replacing hardcoded strings. This improves code readability, maintainability, and reduces the chance of typos.
• Optimized PVC Reconciliation: An optimization has been added to the reconcilePVC function. Before attempting to reconcile a PVC, the function now checks if the PVC associated with the dataset is already in a 'ready' state. If it is, further processing for that PVC is skipped, which reduces redundant operations and improves the efficiency of the reconciliation loop.
• kubeutils Helper Function Update: A new helper function, IsConditionReady, has been added to pkg/kubeutils/kube.go. This function checks if any condition within a provided list of metav1.Condition objects has its Status set to metav1.ConditionTrue. Concurrently, the older ConditionReady function, which checked for a specific condition type's readiness, has been removed.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The code changes aim to fix an issue where a released Persistent Volume (PV) could be incorrectly reused by a new Dataset of type REFERENCE. The fix involves making the PV name unique by appending a prefix of the Dataset's UID. However, I've found a critical bug in the new IsConditionReady helper function, which could lead to incorrect behavior in the reconciliation loop. I've also identified a high-severity issue where the generated PV name could exceed Kubernetes' character limit. My review includes specific suggestions to fix these issues.

[gemini-code-assist]

The cond parameter in IsConditionReady is not used in the function body. This causes the function to check if any condition has Status == metav1.ConditionTrue, rather than checking the specific condition type passed in the cond parameter. This is a critical bug that will lead to incorrect reconciliation logic. The implementation should also check for item.Type == cond.

func IsConditionReady(conditions []metav1.Condition, cond string) bool {
	return lo.ContainsBy(conditions, func(item metav1.Condition) bool {
		return item.Type == cond && item.Status == metav1.ConditionTrue
	})
}

[gemini-code-assist]

The generated PV name could exceed the maximum allowed length for a Kubernetes object name (253 characters for a DNS-1123 subdomain). If ds.Namespace and ds.Name are long, fmt.Sprintf could produce a string that is too long, causing the PV creation to fail. A more robust approach is to truncate the name and append a hash of the full name to ensure it fits within the length limit while maintaining uniqueness.