diff --git a/cmd/cluster/context.go b/cmd/cluster/context.go index 2aa26fa79..71c9d57d0 100644 --- a/cmd/cluster/context.go +++ b/cmd/cluster/context.go @@ -195,6 +195,12 @@ 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. + if o.cluster.Hypershift().Enabled() && o.cluster.Region() != nil && o.cluster.Region().ID() != "" { + o.baseDomain = o.cluster.Region().ID() + } o.infraID = o.cluster.InfraID() if o.usertoken == "" { @@ -374,12 +380,18 @@ 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. + if o.cluster.Hypershift().Enabled() { + pdClientBuilder = pdClientBuilder.WithClusterID(o.clusterID) + } + 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..d236fb479 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 ID. + baseDomain := cluster.DNS().BaseDomain() + var clusterID string + if cluster.Hypershift().Enabled() && cluster.Region() != nil && cluster.Region().ID() != "" { + baseDomain = cluster.Region().ID() + clusterID = cluster.ID() + } + 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..b543d4386 100644 --- a/cmd/org/context_test.go +++ b/cmd/org/context_test.go @@ -125,3 +125,26 @@ func TestFetchContext_ErrorCreateOCMClient(t *testing.T) { t.Errorf("expected error creating OCM client, got %v", err) } } + +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) + } +} diff --git a/pkg/provider/pagerduty/pagerduty.go b/pkg/provider/pagerduty/pagerduty.go index 8c63734e4..9d56a3980 100644 --- a/pkg/provider/pagerduty/pagerduty.go +++ b/pkg/provider/pagerduty/pagerduty.go @@ -33,6 +33,7 @@ type pdClientInterface interface { type client struct { pdclient pdClientInterface baseDomain string + clusterID string teamIds []string userToken string oauthToken string @@ -47,6 +48,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 @@ -106,21 +112,33 @@ func (c *client) GetFiringAlertsForCluster(pdServiceIDs []string) (map[string][] var incidentListOffset uint = 0 for _, pdServiceID := range pdServiceIDs { 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 +149,23 @@ 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 { + 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 diff --git a/pkg/provider/pagerduty/pagerduty_test.go b/pkg/provider/pagerduty/pagerduty_test.go index 5625124ed..85e2a9c3f 100644 --- a/pkg/provider/pagerduty/pagerduty_test.go +++ b/pkg/provider/pagerduty/pagerduty_test.go @@ -20,6 +20,78 @@ func generateIncident() pd.Incident { } } +var _ = Describe("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()) + }) +}) + var _ = Describe("Tests the Pagerduty Provider", func() { var pdProvider *client BeforeEach(func() { @@ -70,6 +142,13 @@ var _ = Describe("Tests the Pagerduty Provider", func() { ctrl.Finish() }) + Context("WithClusterID", func() { + It("Should correctly populate the clusterID", func() { + pdProvider.WithClusterID("test-cluster-123") + Expect(pdProvider.clusterID).To(Equal("test-cluster-123")) + }) + }) + Context("GetPDServiceIDs", func() { It("Returns an error from the pd client if there's an error with the request", func() { m := pdMock.NewMockpdClientInterface(ctrl) @@ -126,6 +205,114 @@ var _ = Describe("Tests the Pagerduty Provider", func() { } }) + 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)) + }) + }) + It("Returns an error from the pd client if there's an error with the request", func() { m := pdMock.NewMockpdClientInterface(ctrl) m.EXPECT().ListIncidentsWithContext(gomock.Any(), gomock.Any()).Return(&pd.ListIncidentsResponse{}, fmt.Errorf("An error"))