GCP-959: Add GCP PSC conditions to metrics and add platform-specific gauges - #9258
GCP-959: Add GCP PSC conditions to metrics and add platform-specific gauges#9258apahim wants to merge 4 commits into
Conversation
Add CredentialStatus type and GetCredentialStatus() function to the GCP platform package, mirroring the existing AWS pattern. The function checks ValidGCPWorkloadIdentity and ValidGCPCredentials conditions and returns a tri-state result: Valid (0), Invalid (1), or Unknown (2). This is the foundation for the new hypershift_cluster_invalid_gcp_creds Prometheus gauge metric. Signed-off-by: Amador Pahim <apahim@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
Add GCPEndpointAvailable and GCPServiceAttachmentAvailable to the GCP case in ExpectedHCConditions(). Both GCP endpoint access modes (Private and PublicAndPrivate) use PSC, so no EndpointAccess gate is needed. This ensures PSC failures appear in the hypershift_hostedclusters_failure_conditions metric. Signed-off-by: Amador Pahim <apahim@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
Add GCPEndpointAvailable and GCPServiceAttachmentAvailable to the condition slice in collectTransitionDurationMetrics(). PSC setup latency is now tracked in the hypershift_hostedclusters_transition_duration histogram. Signed-off-by: Amador Pahim <apahim@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
Add a Prometheus gauge metric that exposes GCP credential validity as a tri-state value: 0=valid, 1=invalid, 2=unknown. This mirrors the existing hypershift_cluster_invalid_aws_creds pattern. The metric is collected unconditionally for all clusters; non-GCP clusters report unknown (2), consistent with the AWS gauge behavior. Signed-off-by: Amador Pahim <apahim@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci-robot
added
the
jira/valid-reference
|
@apahim: This pull request references GCP-959 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThe change adds a GCP credential status type and evaluator. It reports valid, invalid, and unknown credential states through Prometheus metrics. GCP endpoint and service-attachment conditions now contribute to transition-duration metrics and expected condition handling. Tests cover credential evaluation, metric values, and GCP condition transitions. Sequence Diagram(s)sequenceDiagram
participant HostedClusterMetrics
participant HostedCluster
participant GetCredentialStatus
participant PrometheusMetrics
HostedClusterMetrics->>HostedCluster: read GCP conditions
HostedClusterMetrics->>GetCredentialStatus: evaluate credential status
GetCredentialStatus-->>HostedClusterMetrics: return credential status
HostedClusterMetrics->>PrometheusMetrics: emit credential status
HostedClusterMetrics->>PrometheusMetrics: emit condition transition metrics
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: apahim The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-ci
Bot
added
area/control-plane-operator
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@hypershift-operator/controllers/hostedcluster/metrics/metrics.go`:
- Around line 610-619: Update collectGcpCredsMetric in
hypershift-operator/controllers/hostedcluster/metrics/metrics.go:610-619 to
initialize status as platformgcp.CredentialStatusUnknown and call
GetCredentialStatus only when hcluster.Spec.Platform.Type is
hyperv1.GCPPlatform. Update the valid and invalid cases in
hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go:613-630 to
use GCPPlatform, and add a non-GCP case expecting status 2.
🪄 Autofix
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), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 200bcc62-1b99-4108-8637-0de1ee0179dc
📒 Files selected for processing (5)
hypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp.gohypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp_conditions_test.gohypershift-operator/controllers/hostedcluster/metrics/metrics.gohypershift-operator/controllers/hostedcluster/metrics/metrics_test.gosupport/conditions/conditions.go
| // Use detailed credential status: 0=valid, 1=invalid, 2=unknown | ||
| func collectGcpCredsMetric(ch chan<- prometheus.Metric, hcluster *hyperv1.HostedCluster, hclusterLabelValues []string) { | ||
| credStatus := platformgcp.GetCredentialStatus(hcluster) | ||
| ch <- prometheus.MustNewConstMetric( | ||
| invalidGcpCredsMetricDesc, | ||
| prometheus.GaugeValue, | ||
| float64(credStatus), | ||
| hclusterLabelValues..., | ||
| ) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Report unknown for non-GCP HostedClusters.
collectGcpCredsMetric calls platformgcp.GetCredentialStatus for every HostedCluster. A non-GCP HostedCluster can then report 0 or 1 if GCP-named conditions are present. This violates the required metric contract.
hypershift-operator/controllers/hostedcluster/metrics/metrics.go#L610-L619: Initialize the status toplatformgcp.CredentialStatusUnknown. CallGetCredentialStatusonly whenhcluster.Spec.Platform.Type == hyperv1.GCPPlatform.hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go#L613-L630: Set the platform type tohyperv1.GCPPlatformfor valid and invalid cases. Add a non-GCP case that expects2.
📍 Affects 2 files
hypershift-operator/controllers/hostedcluster/metrics/metrics.go#L610-L619(this comment)hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go#L613-L630
🤖 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 `@hypershift-operator/controllers/hostedcluster/metrics/metrics.go` around
lines 610 - 619, Update collectGcpCredsMetric in
hypershift-operator/controllers/hostedcluster/metrics/metrics.go:610-619 to
initialize status as platformgcp.CredentialStatusUnknown and call
GetCredentialStatus only when hcluster.Spec.Platform.Type is
hyperv1.GCPPlatform. Update the valid and invalid cases in
hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go:613-630 to
use GCPPlatform, and add a non-GCP case expecting status 2.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9258 +/- ##
==========================================
+ Coverage 44.81% 45.04% +0.22%
==========================================
Files 775 778 +3
Lines 97219 97576 +357
==========================================
+ Hits 43572 43950 +378
+ Misses 50644 50597 -47
- Partials 3003 3029 +26
... and 37 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
@apahim: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
2 participants
What this PR does / why we need it:
Adds GCP PSC (Private Service Connect) conditions to HyperShift monitoring metrics and introduces a new GCP credential validity gauge, bringing GCP monitoring to parity with AWS.
Changes:
GetCredentialStatus()in GCP platform package — New tri-state function (valid/invalid/unknown) mirroring the AWS pattern, checkingValidGCPWorkloadIdentityandValidGCPCredentialsconditions.PSC conditions in
ExpectedHCConditions()— AddsGCPEndpointAvailableandGCPServiceAttachmentAvailableunconditionally (both GCP endpoint access modes use PSC). PSC failures now appear inhypershift_hostedclusters_failure_conditions.PSC conditions in transition duration metrics — Adds same conditions to
collectTransitionDurationMetrics(). PSC setup latency is now tracked inhypershift_hostedclusters_transition_duration.hypershift_cluster_invalid_gcp_credsgauge — New Prometheus gauge exposing GCP credential status (0=valid, 1=invalid, 2=unknown), following thehypershift_cluster_invalid_aws_credspattern.Which issue(s) this PR fixes:
Fixes https://redhat.atlassian.net/browse/GCP-959
Special notes for your reviewer:
EndpointAccess != Public, but GCP has noPublic-only endpoint access mode (onlyPrivateandPublicAndPrivate), so conditions are added unconditionally.CredentialStatustype is intentionally duplicated from the AWS package to avoid cross-platform import dependencies.Unknown=2), consistent with the AWS gauge behavior.Checklist:
Summary by CodeRabbit