feat(gcp): add GCP PSC conditions to metrics and credential gauge - #9250
feat(gcp): add GCP PSC conditions to metrics and credential gauge#9250apahim wants to merge 5 commits into
Conversation
Add CredentialStatus type and GetCredentialStatus function to the GCP platform package, mirroring the AWS pattern. Unlike the existing ValidCredentials which returns a bool, this provides a tri-state (Valid/Invalid/Unknown) suitable for Prometheus gauge metrics. Refs: GCP-959 Signed-off-by: Amador Pahim <apahim@redhat.com> Co-Authored-By: Claude <noreply@anthropic.com> Commit-Message-Assisted-by: Claude (via Claude Code)
Add GCPEndpointAvailable and GCPServiceAttachmentAvailable to the GCP case in ExpectedHCConditions unconditionally. Both GCP endpoint access modes (Private and PublicAndPrivate) use PSC, so no gate is needed. This ensures PSC failures appear in the hypershift_hostedclusters_failure_conditions metric. Refs: GCP-959 Signed-off-by: Amador Pahim <apahim@redhat.com> Co-Authored-By: Claude <noreply@anthropic.com> Commit-Message-Assisted-by: Claude (via Claude Code)
Add GCPEndpointAvailable and GCPServiceAttachmentAvailable to the collectTransitionDurationMetrics condition slice so that PSC setup latency is recorded in the hypershift_hosted_cluster_transition_seconds histogram. Refs: GCP-959 Signed-off-by: Amador Pahim <apahim@redhat.com> Co-Authored-By: Claude <noreply@anthropic.com> Commit-Message-Assisted-by: Claude (via Claude Code)
Add a Prometheus gauge metric for GCP credential status, mirroring the existing AWS pattern. The metric reports 0=valid, 1=invalid, 2=unknown based on ValidGCPWorkloadIdentity and ValidGCPCredentials conditions. Emitted unconditionally for all clusters (non-GCP clusters report unknown). Refs: GCP-959 Signed-off-by: Amador Pahim <apahim@redhat.com> Co-Authored-By: Claude <noreply@anthropic.com> Commit-Message-Assisted-by: Claude (via Claude Code)
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change adds aggregate GCP credential status evaluation for Workload Identity and credential conditions. It publishes valid, invalid, and unknown states through HostedCluster Prometheus metrics. GCP endpoint and service-attachment conditions now participate in transition-duration metrics. Expected GCP conditions and related tests are updated. Sequence Diagram(s)sequenceDiagram
participant HostedCluster
participant GCPPlatform
participant HostedClusterMetrics
participant Prometheus
HostedCluster->>GCPPlatform: provide credential conditions
GCPPlatform-->>HostedClusterMetrics: return credential status
HostedClusterMetrics->>Prometheus: publish credential-status gauge
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp_conditions_test.go (1)
138-195: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the required test-case description format.
Update each new case description to use
When ... it should ....
hypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp_conditions_test.go#L138-L195: Addit shouldto each credential-status case description.hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go#L656-L695: Addit shouldto each GCP credential metric case description.hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go#L1607-L1609: Change the no-condition case description to use the required format.As per coding guidelines, unit-test case descriptions must use the "When ... it should ..." format.
🤖 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/internal/platform/gcp/gcp_conditions_test.go` around lines 138 - 195, Update the test case descriptions in hypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp_conditions_test.go:138-195 and hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go:656-695 so every GCP credential-status case uses the “When ... it should ...” format; also update the no-condition case in hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go:1607-1609 to follow the same format. Modify only the descriptions, preserving each test’s behavior and expectations.Source: Coding guidelines
🤖 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.
Nitpick comments:
In
`@hypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp_conditions_test.go`:
- Around line 138-195: Update the test case descriptions in
hypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp_conditions_test.go:138-195
and
hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go:656-695 so
every GCP credential-status case uses the “When ... it should ...” format; also
update the no-condition case in
hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go:1607-1609
to follow the same format. Modify only the descriptions, preserving each test’s
behavior and expectations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 43ad4be0-2bde-45be-8fc9-01bb9e758a52
📒 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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9250 +/- ##
==========================================
+ Coverage 44.81% 45.05% +0.23%
==========================================
Files 775 778 +3
Lines 97219 97532 +313
==========================================
+ Hits 43572 43945 +373
+ Misses 50644 50556 -88
- Partials 3003 3031 +28
... and 35 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:
|
Fix gci import ordering in metrics_test.go, add diagnostic message to TestGetCredentialStatus assertion, and add unit tests for ExpectedHCConditions to cover GCP PSC conditions. Co-Authored-By: Claude <noreply@anthropic.com>
|
@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. |
Summary
GCPEndpointAvailableandGCPServiceAttachmentAvailabletoExpectedHCConditions()so PSC failures surface inhypershift_hostedclusters_failure_conditionshypershift_hosted_cluster_transition_secondshistogram for latency visibilityhypershift_cluster_invalid_gcp_credsgauge metric (0=valid, 1=invalid, 2=unknown) mirroring the AWS credential gauge patternGetCredentialStatus()tri-state function to the GCP platform packageTest plan
TestGetCredentialStatus— 8 cases covering valid/invalid/unknown/missing combinationsTestReportInvalidGcpCreds— 7 cases validating gauge metric valuesTestReportTransitionDurationForGCPEndpointConditions— 5 cases validating histogram observationsRefs: GCP-959
🤖 Generated with Claude Code
Summary by CodeRabbit