diff --git a/cmd/cluster/context.go b/cmd/cluster/context.go index 2aa26fa79..c8a20d8c0 100644 --- a/cmd/cluster/context.go +++ b/cmd/cluster/context.go @@ -67,6 +67,7 @@ type contextOptions struct { jiratoken string teamIds []string regionID string + isHCPRegionBased bool } type contextData struct { @@ -195,6 +196,13 @@ func (o *contextOptions) setup() error { o.clusterID = o.cluster.ID() o.externalClusterID = o.cluster.ExternalID() o.baseDomain = o.cluster.DNS().BaseDomain() + // HCP clusters use region-based PD services rather than per-cluster + // services keyed by DNS base domain. Use the region ID as the PD + // service query for HCP clusters. + o.isHCPRegionBased = o.cluster.Hypershift().Enabled() && o.cluster.Region() != nil && o.cluster.Region().ID() != "" + if o.isHCPRegionBased { + o.baseDomain = o.cluster.Region().ID() + } o.infraID = o.cluster.InfraID() if o.usertoken == "" { @@ -374,12 +382,19 @@ func (o *contextOptions) generateContextData() (*contextData, []error) { // For PD query dependencies pdwg := sync.WaitGroup{} var skipPagerDutyCollection bool - pdProvider, err := pagerduty.NewClient(). + pdClientBuilder := pagerduty.NewClient(). WithUserToken(o.usertoken). WithOauthToken(o.oauthtoken). WithBaseDomain(o.baseDomain). - WithTeamIdList(viper.GetStringSlice(pagerduty.PagerDutyTeamIDsKey)). - Init() + WithTeamIdList(viper.GetStringSlice(pagerduty.PagerDutyTeamIDsKey)) + // For HCP clusters, set the cluster ID so PD incidents are filtered + // to only those belonging to this cluster within the region-based + // PD service. Use the external ID because PD alerts reference the + // cluster's external UUID, not the internal OCM ID. + if o.isHCPRegionBased { + pdClientBuilder = pdClientBuilder.WithClusterID(o.externalClusterID) + } + pdProvider, err := pdClientBuilder.Init() if err != nil { skipPagerDutyCollection = true dataErrors = append(dataErrors, fmt.Errorf("skipping PagerDuty context collection: %v", err)) diff --git a/cmd/org/context.go b/cmd/org/context.go index 64756cb08..be4298b5f 100644 --- a/cmd/org/context.go +++ b/cmd/org/context.go @@ -41,7 +41,7 @@ type DefaultContextFetcher struct { GetLimitedSupport func(*sdk.Connection, string) ([]*cmv1.LimitedSupportReason, error) GetServiceLogs func(string, time.Time, bool, bool) ([]*v1.LogEntry, error) GetJiraIssues func(clusterID, externalID, filter string) ([]jira.Issue, error) - NewPDClient func(baseDomain string) (PDClient, error) + NewPDClient func(baseDomain, clusterID string) (PDClient, error) } type PDClient interface { @@ -83,9 +83,10 @@ func NewDefaultContextFetcher() *DefaultContextFetcher { GetLimitedSupport: utils.GetClusterLimitedSupportReasons, GetServiceLogs: servicelog.GetServiceLogsSince, GetJiraIssues: utils.GetJiraIssuesForCluster, - NewPDClient: func(baseDomain string) (PDClient, error) { + NewPDClient: func(baseDomain, clusterID string) (PDClient, error) { return pdProvider.NewClient(). WithBaseDomain(baseDomain). + WithClusterID(clusterID). WithUserToken(viper.GetString(pdProvider.PagerDutyUserTokenConfigKey)). WithOauthToken(viper.GetString(pdProvider.PagerDutyOauthTokenConfigKey)). Init() @@ -207,7 +208,16 @@ func (f *DefaultContextFetcher) FetchContext(orgID string, output io.Writer) ([] }) // PagerDuty alerts dataEg.Go(func() error { - pdClient, err := f.NewPDClient(cluster.DNS().BaseDomain()) + // For HCP clusters, PD services are organized by region + // rather than per-cluster DNS domain. Use the region as + // the query and filter incidents by cluster external ID. + baseDomain := cluster.DNS().BaseDomain() + var clusterID string + if cluster.Hypershift().Enabled() && cluster.Region() != nil && cluster.Region().ID() != "" { + baseDomain = cluster.Region().ID() + clusterID = cluster.ExternalID() + } + pdClient, err := f.NewPDClient(baseDomain, clusterID) if err != nil { return fmt.Errorf("failed to build PD client") } diff --git a/cmd/org/context_test.go b/cmd/org/context_test.go index e2c3c396b..ee77c5e58 100644 --- a/cmd/org/context_test.go +++ b/cmd/org/context_test.go @@ -111,6 +111,29 @@ func TestFetchContext_NoSubscriptions(t *testing.T) { } } +func TestNewPDClient_PassesClusterID(t *testing.T) { + // Verify that the NewPDClient function signature accepts both + // baseDomain and clusterID parameters. + var calledBaseDomain, calledClusterID string + fetcher := &DefaultContextFetcher{ + NewPDClient: func(baseDomain, clusterID string) (PDClient, error) { + calledBaseDomain = baseDomain + calledClusterID = clusterID + return &fakePDClient{ + serviceIDs: []string{"svc-1"}, + incidents: map[string][]pd.Incident{}, + }, nil + }, + } + _, _ = fetcher.NewPDClient("us-east-1", "hcp-cluster-123") + if calledBaseDomain != "us-east-1" { + t.Errorf("expected baseDomain 'us-east-1', got %q", calledBaseDomain) + } + if calledClusterID != "hcp-cluster-123" { + t.Errorf("expected clusterID 'hcp-cluster-123', got %q", calledClusterID) + } +} + func TestFetchContext_ErrorCreateOCMClient(t *testing.T) { fetcher := &DefaultContextFetcher{ SearchSubscriptions: func(orgID string, status string, managedOnly bool) ([]*accountsv1.Subscription, error) { diff --git a/pkg/provider/pagerduty/pagerduty.go b/pkg/provider/pagerduty/pagerduty.go index 8c63734e4..620e09242 100644 --- a/pkg/provider/pagerduty/pagerduty.go +++ b/pkg/provider/pagerduty/pagerduty.go @@ -6,6 +6,7 @@ package pagerduty import ( "context" "fmt" + "os" "sort" "strings" "time" @@ -33,6 +34,7 @@ type pdClientInterface interface { type client struct { pdclient pdClientInterface baseDomain string + clusterID string teamIds []string userToken string oauthToken string @@ -47,6 +49,11 @@ func (c *client) WithBaseDomain(baseDomain string) *client { return c } +func (c *client) WithClusterID(clusterID string) *client { + c.clusterID = clusterID + return c +} + func (c *client) WithTeamIdList(teamIds []string) *client { c.teamIds = teamIds return c @@ -103,24 +110,37 @@ func (c *client) GetFiringAlertsForCluster(pdServiceIDs []string) (map[string][] incidents := map[string][]pd.Incident{} var incidentLimit uint = 25 - var incidentListOffset uint = 0 + var incidentListOffset uint for _, pdServiceID := range pdServiceIDs { + incidentListOffset = 0 for { + opts := pd.ListIncidentsOptions{ + ServiceIDs: []string{pdServiceID}, + Statuses: []string{"triggered", "acknowledged"}, + SortBy: "urgency:DESC", + Limit: incidentLimit, + Offset: incidentListOffset, + } + // For HCP clusters, include first_trigger_log_entries so + // we can filter incidents by cluster ID in EventDetails. + if c.clusterID != "" { + opts.Includes = []string{"first_trigger_log_entries"} + } + listIncidentsResponse, err := c.pdclient.ListIncidentsWithContext( context.TODO(), - pd.ListIncidentsOptions{ - ServiceIDs: []string{pdServiceID}, - Statuses: []string{"triggered", "acknowledged"}, - SortBy: "urgency:DESC", - Limit: incidentLimit, - Offset: incidentListOffset, - }, + opts, ) if err != nil { return nil, err } - incidents[pdServiceID] = append(incidents[pdServiceID], listIncidentsResponse.Incidents...) + for _, incident := range listIncidentsResponse.Incidents { + if c.clusterID != "" && !incidentMatchesCluster(incident, c.clusterID) { + continue + } + incidents[pdServiceID] = append(incidents[pdServiceID], incident) + } if !listIncidentsResponse.More { break @@ -131,6 +151,24 @@ func (c *client) GetFiringAlertsForCluster(pdServiceIDs []string) (map[string][] return incidents, nil } +// incidentMatchesCluster checks whether a PagerDuty incident belongs to the +// given cluster by inspecting the first trigger log entry's EventDetails for +// a matching cluster_id value. This is used for HCP clusters where PD services +// are region-based and contain incidents for multiple clusters. +func incidentMatchesCluster(incident pd.Incident, clusterID string) bool { + ed := incident.FirstTriggerLogEntry.EventDetails + if ed == nil { + fmt.Fprintf(os.Stderr, "Warning: incident %d (%s) has no EventDetails; cannot determine cluster ownership, skipping\n", incident.IncidentNumber, incident.Title) + return false + } + for _, key := range []string{"cluster_id", "clusterID", "cluster-id"} { + if v, ok := ed[key]; ok && v == clusterID { + return true + } + } + return false +} + func (c *client) GetHistoricalAlertsForCluster(pdServiceIDs []string) (map[string][]*IncidentOccurrenceTracker, error) { var currentOffset uint @@ -140,16 +178,24 @@ func (c *client) GetHistoricalAlertsForCluster(pdServiceIDs []string) (map[strin incidentMap := map[string][]*IncidentOccurrenceTracker{} for _, pdServiceID := range pdServiceIDs { + incidents = incidents[:0] for currentOffset = 0; true; currentOffset += limit { + opts := pd.ListIncidentsOptions{ + ServiceIDs: []string{pdServiceID}, + Statuses: []string{"resolved", "triggered", "acknowledged"}, + Offset: currentOffset, + Limit: limit, + SortBy: "created_at:desc", + } + // For HCP clusters, include first_trigger_log_entries so + // we can filter incidents by cluster ID in EventDetails. + if c.clusterID != "" { + opts.Includes = []string{"first_trigger_log_entries"} + } + liResponse, err := c.pdclient.ListIncidentsWithContext( ctx, - pd.ListIncidentsOptions{ - ServiceIDs: []string{pdServiceID}, - Statuses: []string{"resolved", "triggered", "acknowledged"}, - Offset: currentOffset, - Limit: limit, - SortBy: "created_at:desc", - }, + opts, ) if err != nil { @@ -160,7 +206,12 @@ func (c *client) GetHistoricalAlertsForCluster(pdServiceIDs []string) (map[strin break } - incidents = append(incidents, liResponse.Incidents...) + for _, incident := range liResponse.Incidents { + if c.clusterID != "" && !incidentMatchesCluster(incident, c.clusterID) { + continue + } + incidents = append(incidents, incident) + } } incidentCounter := make(map[string]*IncidentOccurrenceTracker) diff --git a/pkg/provider/pagerduty/pagerduty_test.go b/pkg/provider/pagerduty/pagerduty_test.go index 5625124ed..cfa82c791 100644 --- a/pkg/provider/pagerduty/pagerduty_test.go +++ b/pkg/provider/pagerduty/pagerduty_test.go @@ -44,6 +44,12 @@ var _ = Describe("Tests the Pagerduty Provider", func() { Expect(pdProvider.oauthToken).To(Equal("oauth_token")) }) }) + Context("WithClusterID", func() { + It("Should correctly populate the clusterID", func() { + pdProvider.WithClusterID("test-cluster-123") + Expect(pdProvider.clusterID).To(Equal("test-cluster-123")) + }) + }) Context("Building the Client", func() { It("Should build the user_token client when the user client is called", func() { err := pdProvider.WithUserToken("token").buildClient() @@ -70,6 +76,78 @@ var _ = Describe("Tests the Pagerduty Provider", func() { ctrl.Finish() }) + Context("incidentMatchesCluster", func() { + It("Returns true when cluster_id matches", func() { + incident := pd.Incident{ + FirstTriggerLogEntry: pd.FirstTriggerLogEntry{ + CommonLogEntryField: pd.CommonLogEntryField{ + EventDetails: map[string]string{ + "cluster_id": "abc-123", + }, + }, + }, + } + Expect(incidentMatchesCluster(incident, "abc-123")).To(BeTrue()) + }) + + It("Returns true when clusterID key matches", func() { + incident := pd.Incident{ + FirstTriggerLogEntry: pd.FirstTriggerLogEntry{ + CommonLogEntryField: pd.CommonLogEntryField{ + EventDetails: map[string]string{ + "clusterID": "abc-123", + }, + }, + }, + } + Expect(incidentMatchesCluster(incident, "abc-123")).To(BeTrue()) + }) + + It("Returns true when cluster-id key matches", func() { + incident := pd.Incident{ + FirstTriggerLogEntry: pd.FirstTriggerLogEntry{ + CommonLogEntryField: pd.CommonLogEntryField{ + EventDetails: map[string]string{ + "cluster-id": "abc-123", + }, + }, + }, + } + Expect(incidentMatchesCluster(incident, "abc-123")).To(BeTrue()) + }) + + It("Returns false when cluster ID does not match", func() { + incident := pd.Incident{ + FirstTriggerLogEntry: pd.FirstTriggerLogEntry{ + CommonLogEntryField: pd.CommonLogEntryField{ + EventDetails: map[string]string{ + "cluster_id": "different-cluster", + }, + }, + }, + } + Expect(incidentMatchesCluster(incident, "abc-123")).To(BeFalse()) + }) + + It("Returns false when EventDetails is nil", func() { + incident := pd.Incident{} + Expect(incidentMatchesCluster(incident, "abc-123")).To(BeFalse()) + }) + + It("Returns false when no cluster ID key is present", func() { + incident := pd.Incident{ + FirstTriggerLogEntry: pd.FirstTriggerLogEntry{ + CommonLogEntryField: pd.CommonLogEntryField{ + EventDetails: map[string]string{ + "some_other_key": "abc-123", + }, + }, + }, + } + Expect(incidentMatchesCluster(incident, "abc-123")).To(BeFalse()) + }) + }) + Context("GetPDServiceIDs", func() { It("Returns an error from the pd client if there's an error with the request", func() { m := pdMock.NewMockpdClientInterface(ctrl) @@ -190,6 +268,189 @@ var _ = Describe("Tests the Pagerduty Provider", func() { Expect(incs["baz"]).To(BeEmpty()) }) }) + + Context("HCP cluster ID filtering", func() { + It("Returns only incidents matching the cluster ID in EventDetails", func() { + matchingIncident := pd.Incident{ + IncidentNumber: 1, + Title: "MatchingAlert", + FirstTriggerLogEntry: pd.FirstTriggerLogEntry{ + CommonLogEntryField: pd.CommonLogEntryField{ + EventDetails: map[string]string{ + "cluster_id": "hcp-cluster-123", + }, + }, + }, + } + nonMatchingIncident := pd.Incident{ + IncidentNumber: 2, + Title: "OtherClusterAlert", + FirstTriggerLogEntry: pd.FirstTriggerLogEntry{ + CommonLogEntryField: pd.CommonLogEntryField{ + EventDetails: map[string]string{ + "cluster_id": "hcp-cluster-999", + }, + }, + }, + } + noDetailsIncident := pd.Incident{ + IncidentNumber: 3, + Title: "NoDetailsAlert", + } + mixedResponse := &pd.ListIncidentsResponse{ + Incidents: []pd.Incident{matchingIncident, nonMatchingIncident, noDetailsIncident}, + } + + m := pdMock.NewMockpdClientInterface(ctrl) + m.EXPECT().ListIncidentsWithContext(gomock.Any(), gomock.Any()).Return(mixedResponse, nil) + pdProvider.pdclient = m + pdProvider.clusterID = "hcp-cluster-123" + + incs, err := pdProvider.GetFiringAlertsForCluster([]string{"region-svc"}) + Expect(err).To(BeNil()) + Expect(incs["region-svc"]).To(HaveLen(1)) + Expect(incs["region-svc"][0].Title).To(Equal("MatchingAlert")) + }) + + It("Returns empty when no incidents match the cluster ID", func() { + nonMatchingResponse := &pd.ListIncidentsResponse{ + Incidents: []pd.Incident{ + { + IncidentNumber: 1, + Title: "OtherAlert", + FirstTriggerLogEntry: pd.FirstTriggerLogEntry{ + CommonLogEntryField: pd.CommonLogEntryField{ + EventDetails: map[string]string{ + "cluster_id": "different-cluster", + }, + }, + }, + }, + }, + } + + m := pdMock.NewMockpdClientInterface(ctrl) + m.EXPECT().ListIncidentsWithContext(gomock.Any(), gomock.Any()).Return(nonMatchingResponse, nil) + pdProvider.pdclient = m + pdProvider.clusterID = "hcp-cluster-123" + + incs, err := pdProvider.GetFiringAlertsForCluster([]string{"region-svc"}) + Expect(err).To(BeNil()) + Expect(incs["region-svc"]).To(BeEmpty()) + }) + + It("Supports alternate cluster ID key names", func() { + incident := pd.Incident{ + IncidentNumber: 1, + Title: "AlternateKeyAlert", + FirstTriggerLogEntry: pd.FirstTriggerLogEntry{ + CommonLogEntryField: pd.CommonLogEntryField{ + EventDetails: map[string]string{ + "clusterID": "hcp-cluster-alt", + }, + }, + }, + } + response := &pd.ListIncidentsResponse{ + Incidents: []pd.Incident{incident}, + } + + m := pdMock.NewMockpdClientInterface(ctrl) + m.EXPECT().ListIncidentsWithContext(gomock.Any(), gomock.Any()).Return(response, nil) + pdProvider.pdclient = m + pdProvider.clusterID = "hcp-cluster-alt" + + incs, err := pdProvider.GetFiringAlertsForCluster([]string{"region-svc"}) + Expect(err).To(BeNil()) + Expect(incs["region-svc"]).To(HaveLen(1)) + }) + + It("Does not filter when clusterID is empty (classic cluster behavior)", func() { + m := pdMock.NewMockpdClientInterface(ctrl) + m.EXPECT().ListIncidentsWithContext(gomock.Any(), gomock.Any()).Return(singleIncResponse, nil) + pdProvider.pdclient = m + pdProvider.clusterID = "" + + incs, err := pdProvider.GetFiringAlertsForCluster([]string{"classic-svc"}) + Expect(err).To(BeNil()) + Expect(incs["classic-svc"]).To(HaveLen(1)) + }) + }) + }) + + Context("GetHistoricalAlertsForCluster HCP filtering", func() { + It("Returns only incidents matching the cluster ID in historical data", func() { + matchingIncident := pd.Incident{ + IncidentNumber: 1, + Title: "MatchingAlert is firing", + CreatedAt: "2024-01-15T10:00:00Z", + FirstTriggerLogEntry: pd.FirstTriggerLogEntry{ + CommonLogEntryField: pd.CommonLogEntryField{ + EventDetails: map[string]string{ + "cluster_id": "hcp-cluster-123", + }, + }, + }, + } + nonMatchingIncident := pd.Incident{ + IncidentNumber: 2, + Title: "OtherClusterAlert is firing", + CreatedAt: "2024-01-15T11:00:00Z", + FirstTriggerLogEntry: pd.FirstTriggerLogEntry{ + CommonLogEntryField: pd.CommonLogEntryField{ + EventDetails: map[string]string{ + "cluster_id": "hcp-cluster-999", + }, + }, + }, + } + mixedResponse := &pd.ListIncidentsResponse{ + Incidents: []pd.Incident{matchingIncident, nonMatchingIncident}, + } + emptyResponse := &pd.ListIncidentsResponse{ + Incidents: []pd.Incident{}, + } + + m := pdMock.NewMockpdClientInterface(ctrl) + m.EXPECT().ListIncidentsWithContext(gomock.Any(), gomock.Any()).Return(mixedResponse, nil) + m.EXPECT().ListIncidentsWithContext(gomock.Any(), gomock.Any()).Return(emptyResponse, nil) + pdProvider.pdclient = m + pdProvider.clusterID = "hcp-cluster-123" + + result, err := pdProvider.GetHistoricalAlertsForCluster([]string{"region-svc"}) + Expect(err).To(BeNil()) + Expect(result["region-svc"]).To(HaveLen(1)) + Expect(result["region-svc"][0].IncidentName).To(Equal("MatchingAlert")) + }) + + It("Returns all incidents when clusterID is empty (classic cluster)", func() { + incident1 := pd.Incident{ + IncidentNumber: 1, + Title: "Alert1 is firing", + CreatedAt: "2024-01-15T10:00:00Z", + } + incident2 := pd.Incident{ + IncidentNumber: 2, + Title: "Alert2 is firing", + CreatedAt: "2024-01-15T11:00:00Z", + } + response := &pd.ListIncidentsResponse{ + Incidents: []pd.Incident{incident1, incident2}, + } + emptyResponse := &pd.ListIncidentsResponse{ + Incidents: []pd.Incident{}, + } + + m := pdMock.NewMockpdClientInterface(ctrl) + m.EXPECT().ListIncidentsWithContext(gomock.Any(), gomock.Any()).Return(response, nil) + m.EXPECT().ListIncidentsWithContext(gomock.Any(), gomock.Any()).Return(emptyResponse, nil) + pdProvider.pdclient = m + pdProvider.clusterID = "" + + result, err := pdProvider.GetHistoricalAlertsForCluster([]string{"classic-svc"}) + Expect(err).To(BeNil()) + Expect(result["classic-svc"]).To(HaveLen(2)) + }) }) }) })