AIPCC:15489: Add support for Gaudi accelerator in mapt's IBM Cloud module#834
Open
deekay2310 wants to merge 5 commits into
Open
AIPCC:15489: Add support for Gaudi accelerator in mapt's IBM Cloud module#834deekay2310 wants to merge 5 commits into
deekay2310 wants to merge 5 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds IBM Gaudi3 provisioning and teardown support through a new ChangesIBM Gaudi3 provisioning feature
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
deekay2310
changed the title
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
pkg/provider/ibmcloud/action/ibm-gaudi/cloud-config (1)
94-98: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUnquoted user-controlled key/value in extra attrs.
{{$k}}and{{$v}}fromOtelExtraAttrs(user-supplied via--otel-extra-attrs) are interpolated into YAML unquoted/partially quoted. A key containing YAML-significant characters (:,#, newline) could corrupt the generated config or inject unintended fields.♻️ Proposed fix
{{- range $k, $v := .OtelExtraAttrs}} - - key: {{$k}} + - key: "{{$k}}" value: "{{$v}}" action: upsert {{- end}}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/provider/ibmcloud/action/ibm-gaudi/cloud-config` around lines 94 - 98, The OtelExtraAttrs entries in the cloud-config template are being interpolated into YAML without safely quoting the user-controlled key and value, which can break the generated config or allow injected fields. Update the template loop in the cloud-config rendering block so the OtelExtraAttrs key/value are emitted using YAML-safe quoting/escaping, preserving the existing upsert structure while preventing special characters from being interpreted as YAML syntax.pkg/provider/ibmcloud/action/ibm-gaudi/ibm-gaudi.go (1)
173-233: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftLarge duplication between
deployanddeployWithExistingSubnet.Instance-args assembly, OTEL user-data injection (188-194 / 285-291), floating-IP association (202-215 / 299-312), username/host exports, and the readiness
remote.NewCommandblock (217-231 / 314-328) are nearly identical. Extracting a shared helper (e.g. taking VPC/zone/subnet/floating-IP/security-group inputs) would reduce drift risk as this evolves.Also applies to: 273-330
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/provider/ibmcloud/action/ibm-gaudi/ibm-gaudi.go` around lines 173 - 233, There is significant duplication between the deploy paths, especially around instanceArgs setup, OTEL user-data injection in gaudiUserData handling, floating-IP association with ibmcloud.NewIsInstanceNetworkInterfaceFloatingIp, ctx.Export of username/host, and the remote.NewCommand readiness check. Extract the shared provisioning logic into a helper used by both deploy and deployWithExistingSubnet, parameterized by the varying inputs like VPC, zone, subnet, security group, and floating IP, so future changes only need to be made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/ibmcloud/ibm-gaudi.md`:
- Around line 60-64: Update the IBM Cloud Gaudi SSH documentation to use the
current default SSH user `cloud-user` instead of `root`. In the table entry for
`username` and in the SSH access example, replace the stale `root` references
with `cloud-user` so the docs match the current RHEL AI image behavior. Use the
existing SSH example section and the `host`/`username` table to locate and
update both places consistently.
In `@pkg/provider/ibmcloud/action/ibm-gaudi/cloud-config`:
- Around line 99-103: The otlphttp exporter in the IBM Gaudi cloud-config
disables TLS verification with insecure_skip_verify, which should be removed.
Update the exporter configuration to use proper certificate validation for the
OTEL endpoint in this cloud-config block, and if custom trust is needed, wire in
the appropriate CA/cert settings instead of bypassing verification.
- Around line 128-144: The otelcol-contrib cloud-init install flow in the runcmd
block has no failure stopping behavior, so a bad download or RPM install can be
ignored and the script keeps going. Add explicit error handling to this shell
fragment (for example, stop on failures and check the curl and rpm steps before
proceeding) so the install only continues after each command succeeds. Use the
runcmd shell block and the curl/rpm install sequence as the key points to
update.
In `@pkg/provider/ibmcloud/action/ibm-gaudi/ibm-gaudi.go`:
- Around line 188-194: The OTEL user-data gating in gaudi user data generation
is broader than the cloud-config template, so the collector install can be
skipped while config files are still written. Update the condition around
gaudiUserData/UserData assignment in the IBM Gaudi flow to match the template’s
requirements, including otelIndex, or adjust the template to match the intended
behavior. Check the gaudiUserData helper and the instance setup block to keep
the gating consistent in both places.
- Around line 382-388: Stop logging the generated private key in debug output;
the current ApplyT callback on pk.PrivateKeyPem in the ibm-gaudi flow prints
sensitive material. Update the debug branch to log a non-sensitive identifier
instead, such as the public key or a fingerprint, and apply the same fix in the
analogous ApplyT/debug logging blocks in ibm-power and ibm-z so no private key
values are emitted anywhere.
---
Nitpick comments:
In `@pkg/provider/ibmcloud/action/ibm-gaudi/cloud-config`:
- Around line 94-98: The OtelExtraAttrs entries in the cloud-config template are
being interpolated into YAML without safely quoting the user-controlled key and
value, which can break the generated config or allow injected fields. Update the
template loop in the cloud-config rendering block so the OtelExtraAttrs
key/value are emitted using YAML-safe quoting/escaping, preserving the existing
upsert structure while preventing special characters from being interpreted as
YAML syntax.
In `@pkg/provider/ibmcloud/action/ibm-gaudi/ibm-gaudi.go`:
- Around line 173-233: There is significant duplication between the deploy
paths, especially around instanceArgs setup, OTEL user-data injection in
gaudiUserData handling, floating-IP association with
ibmcloud.NewIsInstanceNetworkInterfaceFloatingIp, ctx.Export of username/host,
and the remote.NewCommand readiness check. Extract the shared provisioning logic
into a helper used by both deploy and deployWithExistingSubnet, parameterized by
the varying inputs like VPC, zone, subnet, security group, and floating IP, so
future changes only need to be made in one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: d85efaa9-9a57-456b-b2de-bdc9fbb4fc4f
📒 Files selected for processing (5)
cmd/mapt/cmd/ibmcloud/hosts/ibm-gaudi.gocmd/mapt/cmd/ibmcloud/ibmcloud.godocs/ibmcloud/ibm-gaudi.mdpkg/provider/ibmcloud/action/ibm-gaudi/cloud-configpkg/provider/ibmcloud/action/ibm-gaudi/ibm-gaudi.go
Comment on lines
+128
to
+144
| runcmd: | ||
| - | | ||
| PROXY_URL="" | ||
| if ! curl -sf --connect-timeout 5 --head {{.OtelEndpoint}} > /dev/null 2>&1; then | ||
| PROXY_URL="http://squid.corp.redhat.com:3128" | ||
| fi | ||
| RPM_URL="https://github.com/open-telemetry/opentelemetry-collector-releases/releases/download/v{{.OtelColVersion}}/otelcol-contrib_{{.OtelColVersion}}_linux_{{.OtelArch}}.rpm" | ||
| HTTPS_PROXY="$PROXY_URL" curl -fsSL -o /tmp/otelcol-contrib.rpm "$RPM_URL" | ||
| rpm -Uvh /tmp/otelcol-contrib.rpm | ||
| rm -f /tmp/otelcol-contrib.rpm | ||
| chown -R otelcol-contrib:otelcol-contrib /etc/otelcol-contrib | ||
| if [ -n "$PROXY_URL" ]; then | ||
| printf '[Service]\nEnvironment="HTTPS_PROXY=%s/"\nEnvironment="NO_PROXY=10.*,192.168.*,localhost,127.0.0.1"\n' "$PROXY_URL" \ | ||
| > /etc/systemd/system/otelcol-contrib.service.d/proxy.conf | ||
| fi | ||
| systemctl daemon-reload | ||
| systemctl enable --now otelcol-contrib |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
No error handling in runcmd install script.
Lack of set -e/explicit checks after curl and rpm -Uvh means a failed download silently proceeds to install a stale/missing RPM, potentially leaving the instance without a functioning otelcol-contrib service and no clear failure signal in cloud-init logs.
🛡️ Proposed fix
- |
+ set -e
PROXY_URL=""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| runcmd: | |
| - | | |
| PROXY_URL="" | |
| if ! curl -sf --connect-timeout 5 --head {{.OtelEndpoint}} > /dev/null 2>&1; then | |
| PROXY_URL="http://squid.corp.redhat.com:3128" | |
| fi | |
| RPM_URL="https://github.com/open-telemetry/opentelemetry-collector-releases/releases/download/v{{.OtelColVersion}}/otelcol-contrib_{{.OtelColVersion}}_linux_{{.OtelArch}}.rpm" | |
| HTTPS_PROXY="$PROXY_URL" curl -fsSL -o /tmp/otelcol-contrib.rpm "$RPM_URL" | |
| rpm -Uvh /tmp/otelcol-contrib.rpm | |
| rm -f /tmp/otelcol-contrib.rpm | |
| chown -R otelcol-contrib:otelcol-contrib /etc/otelcol-contrib | |
| if [ -n "$PROXY_URL" ]; then | |
| printf '[Service]\nEnvironment="HTTPS_PROXY=%s/"\nEnvironment="NO_PROXY=10.*,192.168.*,localhost,127.0.0.1"\n' "$PROXY_URL" \ | |
| > /etc/systemd/system/otelcol-contrib.service.d/proxy.conf | |
| fi | |
| systemctl daemon-reload | |
| systemctl enable --now otelcol-contrib | |
| runcmd: | |
| - | | |
| set -e | |
| PROXY_URL="" | |
| if ! curl -sf --connect-timeout 5 --head {{.OtelEndpoint}} > /dev/null 2>&1; then | |
| PROXY_URL="http://squid.corp.redhat.com:3128" | |
| fi | |
| RPM_URL="https://github.com/open-telemetry/opentelemetry-collector-releases/releases/download/v{{.OtelColVersion}}/otelcol-contrib_{{.OtelColVersion}}_linux_{{.OtelArch}}.rpm" | |
| HTTPS_PROXY="$PROXY_URL" curl -fsSL -o /tmp/otelcol-contrib.rpm "$RPM_URL" | |
| rpm -Uvh /tmp/otelcol-contrib.rpm | |
| rm -f /tmp/otelcol-contrib.rpm | |
| chown -R otelcol-contrib:otelcol-contrib /etc/otelcol-contrib | |
| if [ -n "$PROXY_URL" ]; then | |
| printf '[Service]\nEnvironment="HTTPS_PROXY=%s/"\nEnvironment="NO_PROXY=10.*,192.168.*,localhost,127.0.0.1"\n' "$PROXY_URL" \ | |
| > /etc/systemd/system/otelcol-contrib.service.d/proxy.conf | |
| fi | |
| systemctl daemon-reload | |
| systemctl enable --now otelcol-contrib |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/provider/ibmcloud/action/ibm-gaudi/cloud-config` around lines 128 - 144,
The otelcol-contrib cloud-init install flow in the runcmd block has no failure
stopping behavior, so a bad download or RPM install can be ignored and the
script keeps going. Add explicit error handling to this shell fragment (for
example, stop on failures and check the curl and rpm steps before proceeding) so
the install only continues after each command succeeds. Use the runcmd shell
block and the curl/rpm install sequence as the key points to update.
adrianriobo
requested changes
Jul 3, 2026
| //go:embed cloud-config | ||
| var CloudConfig []byte | ||
|
|
||
| var otelColVersion = "0.151.0" |
Collaborator
There was a problem hiding this comment.
why is this here? Did you rebase the branch, now whole otel is managed through its own intetration package
Contributor
Author
There was a problem hiding this comment.
Rebased onto latest main and refactored to use pkg/integrations/otelcol. Removed inline otelColVersion, the OTEL-specific userDataValues fields, and the 140-line inline cloud-config. Now uses otelcol.GetSnippetAsCloudInitWritableFile() matching the pattern in ibm-power and ibm-z.
| outputUserPrivateKey = "icgUserPrivatekey" | ||
|
|
||
| defaultProfile = "gx3d-160x1792x8gaudi3" | ||
| defaultImage = "ibm-redhat-ai-intel" |
Contributor
Author
There was a problem hiding this comment.
This is the RHEL AI stock image for Intel/x86_64 VPC instances. It resolves via GetVPCImage which uses strings.Contains (substring match), so ibm-redhat-ai-intel matches the full image name:
$ ibmcloud is images --visibility public | grep -i "redhat-ai"
ibm-redhat-ai-intel-1-5-1-amd64-1 available public
ibm-redhat-ai-nvidia-1-5-2-amd64-1 available public
Added a comment in the code noting the substring match behavior and the resolved name.
Address PR review: replace inline OTEL config with pkg/integrations/otelcol, add image name comment, update docs SSH username to cloud-user. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
deekay2310
force-pushed
the
ibmcloud-gaudi3
branch
from
c4577bb to
06994bd
Compare
… check DialErrorLimit defaults to 10 (exhausted in ~23s), too short for Gaudi 3 instances which take minutes to boot. Set to -1 (unlimited, bounded by the 40m RemoteTimeout) matching the pattern across all other providers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
ibmcloud ibm-gaudicreate/destroy commands for provisioning Intel Gaudi 3 accelerator instances on IBM Cloud VPC.ibm-gaudiwith auto-provisioning via RHEL AI image ongx3d-160x1792x8profile (us-south-2)cloud-useras default SSH user for the RHEL AI imageResolves: AIPCC-15489