fix(slurm): robustly convert enable_oslogin boolean to metadata string#5351
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! 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 resolves a critical bug in the Slurm blueprint's OS Login configuration. Previously, attempts to disable OS Login through the blueprint would often fail, resulting in OS Login remaining enabled on provisioned nodes due to an unreliable boolean-to-string conversion. The implemented fix ensures that the Highlights
Changelog
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. 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
|
There was a problem hiding this comment.
Code Review
This pull request modifies the conversion of the enable_oslogin boolean variable to a string for instance metadata. The implementation is changed from upper(var.enable_oslogin) to var.enable_oslogin ? "TRUE" : "FALSE" to ensure a reliable conversion. After reviewing the changes, I have no specific comments or suggestions for improvement.
89af64b to
4024285
Compare
71695be
into
GoogleCloudPlatform:develop
Problem
When disabling OS Login with
enable_oslogin: falsein a Slurm blueprint, the mapping into custom metadata on provisioned nodes was defaulting or evaluated back to"TRUE".The internal Slurm mapping was using the Terraform
upper()function on a boolean variable (enable-oslogin = upper(var.enable_oslogin)). This implicit conversion often falls back on previous casing or fails, causing it to remain"TRUE".Solution
Changed
upper(var.enable_oslogin)to an explicit ternary conditional:enable-oslogin = var.enable_oslogin ? "TRUE" : "FALSE"This ensures that any boolean value (
true/false) evaluated from your blueprint is explicitly and reliably translated into the expected"TRUE"or"FALSE"metadata format required by Compute Engine.