Skip to content

[SDCICD-1812] update region if cluster id is provided#3207

Merged
openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
varunraokadaparthi:SDCICD-1812
Apr 10, 2026
Merged

[SDCICD-1812] update region if cluster id is provided#3207
openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
varunraokadaparthi:SDCICD-1812

Conversation

@varunraokadaparthi
Copy link
Copy Markdown
Contributor

fix for region not set error in jenkins job
Co-authored-by: Cursor cursor@cursor.com

@openshift-merge-bot
Copy link
Copy Markdown
Contributor

There are test jobs defined for this repository which are not configured to run automatically. Comment /test ? to see a list of all defined jobs. Review these jobs and use /test <job> to manually trigger jobs most likely to be impacted by the proposed changes.Comment /pipeline required to trigger all required & necessary jobs.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 10, 2026

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: d24e6675-6bb5-4f54-adeb-96abbc792558

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

The changes introduce a new GetClusterRegion(clusterID string) (string, error) method across the provider abstraction layer (interface, OCM, and ROSA providers), enabling cluster region retrieval. The method is integrated into KrknAI initialization to resolve and configure the AWS region, with a corresponding script environment variable update.

Changes

Cohort / File(s) Summary
Provider Interface & Implementations
pkg/common/spi/provider.go, pkg/common/providers/ocmprovider/cluster.go, pkg/common/providers/rosaprovider/wrapped_calls.go
Added GetClusterRegion method signature to Provider interface; implemented in OCMProvider (delegates to GetCluster and extracts region with error handling for missing regions); implemented in ROSAProvider (delegates to ocmProvider and updates awsRegion field on success).
Integration & Deployment
pkg/krknai/krknai.go, scripts/parameterized-job-krknai.sh
Conditionally calls provider.GetClusterRegion() in KrknAI.New() when cluster ID is set, updates viper config with resolved region; script now sets AWS_REGION=us-east-1 environment variable in container execution.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Apr 10, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: varunraokadaparthi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 10, 2026
@varunraokadaparthi
Copy link
Copy Markdown
Contributor Author

@coderabbitai review
/test hypershift-pr-check

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 10, 2026

@varunraokadaparthi: Initiating a review of the PR.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
pkg/krknai/krknai.go (1)

66-73: Keep region keys synchronized after cluster lookup.

Line 72 updates only config.AWSRegion. AnalyzeLogs reads config.CloudProvider.Region (Line 438), so region state can diverge. Set both keys on success.

Suggested patch
 		viper.Set(config.AWSRegion, clusterRegion)
+		viper.Set(config.CloudProvider.Region, clusterRegion)
 	}

As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/krknai/krknai.go` around lines 66 - 73, The code sets viper key
config.AWSRegion after fetching clusterRegion via provider.GetClusterRegion but
AnalyzeLogs reads config.CloudProvider.Region, so keep both keys synchronized:
after successful GetClusterRegion in the block around
viper.Set(config.AWSRegion, clusterRegion), also set
viper.Set(config.CloudProvider.Region, clusterRegion) (and ensure any other
consumers expect the same key); update the cluster lookup block that calls
provider.GetClusterRegion to set both config.AWSRegion and
config.CloudProvider.Region to the returned clusterRegion so AnalyzeLogs and
other code read the same region value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/parameterized-job-krknai.sh`:
- Line 63: The job runner currently hard-codes AWS region via the literal
AWS_REGION="us-east-1" in scripts/parameterized-job-krknai.sh; replace that
hard-coded value with a caller-provided environment variable fallback (e.g., use
shell parameter expansion for AWS_REGION with a default) so callers can override
the region without editing the script and the runner still has a sensible
default; update any references that set or export AWS_REGION in the script
(search for AWS_REGION) and ensure the script documents/accepts an env var or
argument for region.

---

Nitpick comments:
In `@pkg/krknai/krknai.go`:
- Around line 66-73: The code sets viper key config.AWSRegion after fetching
clusterRegion via provider.GetClusterRegion but AnalyzeLogs reads
config.CloudProvider.Region, so keep both keys synchronized: after successful
GetClusterRegion in the block around viper.Set(config.AWSRegion, clusterRegion),
also set viper.Set(config.CloudProvider.Region, clusterRegion) (and ensure any
other consumers expect the same key); update the cluster lookup block that calls
provider.GetClusterRegion to set both config.AWSRegion and
config.CloudProvider.Region to the returned clusterRegion so AnalyzeLogs and
other code read the same region value.
🪄 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: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: c7c2eb5b-d7b3-4488-bba5-e073fcee35e1

📥 Commits

Reviewing files that changed from the base of the PR and between ac20e6b and 15d8083.

📒 Files selected for processing (5)
  • pkg/common/providers/ocmprovider/cluster.go
  • pkg/common/providers/rosaprovider/wrapped_calls.go
  • pkg/common/spi/provider.go
  • pkg/krknai/krknai.go
  • scripts/parameterized-job-krknai.sh

@ritmun
Copy link
Copy Markdown
Contributor

ritmun commented Apr 10, 2026

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 10, 2026
@varunraokadaparthi
Copy link
Copy Markdown
Contributor Author

/override ci/prow/hypershift-pr-check

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Apr 10, 2026

@varunraokadaparthi: Overrode contexts on behalf of varunraokadaparthi: ci/prow/hypershift-pr-check

Details

In response to this:

/override ci/prow/hypershift-pr-check

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Apr 10, 2026

@varunraokadaparthi: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit 686d53c into openshift:main Apr 10, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants