Skip to content

Commit 11f7b1b

Browse files
dannykoppingdannykopping
authored
chore: remove notifications experiment (#14869)
Notifications have proved stable in the [mainline release of v2.15](https://github.com/coder/coder/releases/tag/v2.15.0), and in preparation for v2.16 we're moving this to stable.
1 parent edb4485 commit 11f7b1b

File tree

17 files changed

+95
-90
lines changed

17 files changed

+95
-90
lines changed

‎cli/notifications_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ func createOpts(t *testing.T) *coderdtest.Options {
2020
t.Helper()
2121

2222
dt := coderdtest.DeploymentValues(t)
23-
dt.Experiments = []string{string(codersdk.ExperimentNotifications)}
2423
return &coderdtest.Options{
2524
DeploymentValues: dt,
2625
}

‎cli/server.go

Lines changed: 42 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,16 @@ import (
5656

5757
"cdr.dev/slog"
5858
"cdr.dev/slog/sloggers/sloghuman"
59-
"github.com/coder/coder/v2/coderd/entitlements"
60-
"github.com/coder/coder/v2/coderd/notifications/reports"
61-
"github.com/coder/coder/v2/coderd/runtimeconfig"
6259
"github.com/coder/pretty"
6360
"github.com/coder/quartz"
6461
"github.com/coder/retry"
6562
"github.com/coder/serpent"
6663
"github.com/coder/wgtunnel/tunnelsdk"
6764

65+
"github.com/coder/coder/v2/coderd/entitlements"
66+
"github.com/coder/coder/v2/coderd/notifications/reports"
67+
"github.com/coder/coder/v2/coderd/runtimeconfig"
68+
6869
"github.com/coder/coder/v2/buildinfo"
6970
"github.com/coder/coder/v2/cli/clilog"
7071
"github.com/coder/coder/v2/cli/cliui"
@@ -684,10 +685,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
684685
options.OIDCConfig = oc
685686
}
686687

687-
experiments := coderd.ReadExperiments(
688-
options.Logger, options.DeploymentValues.Experiments.Value(),
689-
)
690-
691688
// We'll read from this channel in the select below that tracks shutdown. If it remains
692689
// nil, that case of the select will just never fire, but it's important not to have a
693690
// "bare" read on this channel.
@@ -951,6 +948,33 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
951948
return xerrors.Errorf("write config url: %w", err)
952949
}
953950

951+
// Manage notifications.
952+
cfg := options.DeploymentValues.Notifications
953+
metrics := notifications.NewMetrics(options.PrometheusRegistry)
954+
helpers := templateHelpers(options)
955+
956+
// The enqueuer is responsible for enqueueing notifications to the given store.
957+
enqueuer, err := notifications.NewStoreEnqueuer(cfg, options.Database, helpers, logger.Named("notifications.enqueuer"), quartz.NewReal())
958+
if err != nil {
959+
return xerrors.Errorf("failed to instantiate notification store enqueuer: %w", err)
960+
}
961+
options.NotificationsEnqueuer = enqueuer
962+
963+
// The notification manager is responsible for:
964+
// - creating notifiers and managing their lifecycles (notifiers are responsible for dequeueing/sending notifications)
965+
// - keeping the store updated with status updates
966+
notificationsManager, err := notifications.NewManager(cfg, options.Database, helpers, metrics, logger.Named("notifications.manager"))
967+
if err != nil {
968+
return xerrors.Errorf("failed to instantiate notification manager: %w", err)
969+
}
970+
971+
// nolint:gocritic // TODO: create own role.
972+
notificationsManager.Run(dbauthz.AsSystemRestricted(ctx))
973+
974+
// Run report generator to distribute periodic reports.
975+
notificationReportGenerator := reports.NewReportGenerator(ctx, logger.Named("notifications.report_generator"), options.Database, options.NotificationsEnqueuer, quartz.NewReal())
976+
defer notificationReportGenerator.Close()
977+
954978
// Since errCh only has one buffered slot, all routines
955979
// sending on it must be wrapped in a select/default to
956980
// avoid leaving dangling goroutines waiting for the
@@ -1007,38 +1031,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
10071031
options.WorkspaceUsageTracker = tracker
10081032
defer tracker.Close()
10091033

1010-
// Manage notifications.
1011-
var (
1012-
notificationsManager *notifications.Manager
1013-
)
1014-
if experiments.Enabled(codersdk.ExperimentNotifications) {
1015-
cfg := options.DeploymentValues.Notifications
1016-
metrics := notifications.NewMetrics(options.PrometheusRegistry)
1017-
helpers := templateHelpers(options)
1018-
1019-
// The enqueuer is responsible for enqueueing notifications to the given store.
1020-
enqueuer, err := notifications.NewStoreEnqueuer(cfg, options.Database, helpers, logger.Named("notifications.enqueuer"), quartz.NewReal())
1021-
if err != nil {
1022-
return xerrors.Errorf("failed to instantiate notification store enqueuer: %w", err)
1023-
}
1024-
options.NotificationsEnqueuer = enqueuer
1025-
1026-
// The notification manager is responsible for:
1027-
// - creating notifiers and managing their lifecycles (notifiers are responsible for dequeueing/sending notifications)
1028-
// - keeping the store updated with status updates
1029-
notificationsManager, err = notifications.NewManager(cfg, options.Database, helpers, metrics, logger.Named("notifications.manager"))
1030-
if err != nil {
1031-
return xerrors.Errorf("failed to instantiate notification manager: %w", err)
1032-
}
1033-
1034-
// nolint:gocritic // TODO: create own role.
1035-
notificationsManager.Run(dbauthz.AsSystemRestricted(ctx))
1036-
1037-
// Run report generator to distribute periodic reports.
1038-
notificationReportGenerator := reports.NewReportGenerator(ctx, logger.Named("notifications.report_generator"), options.Database, options.NotificationsEnqueuer, quartz.NewReal())
1039-
defer notificationReportGenerator.Close()
1040-
}
1041-
10421034
// Wrap the server in middleware that redirects to the access URL if
10431035
// the request is not to a local IP.
10441036
var handler http.Handler = coderAPI.RootHandler
@@ -1158,19 +1150,17 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
11581150
// Cancel any remaining in-flight requests.
11591151
shutdownConns()
11601152

1161-
if notificationsManager != nil {
1162-
// Stop the notification manager, which will cause any buffered updates to the store to be flushed.
1163-
// If the Stop() call times out, messages that were sent but not reflected as such in the store will have
1164-
// their leases expire after a period of time and will be re-queued for sending.
1165-
// See CODER_NOTIFICATIONS_LEASE_PERIOD.
1166-
cliui.Info(inv.Stdout, "Shutting down notifications manager..."+"\n")
1167-
err = shutdownWithTimeout(notificationsManager.Stop, 5*time.Second)
1168-
if err != nil {
1169-
cliui.Warnf(inv.Stderr, "Notifications manager shutdown took longer than 5s, "+
1170-
"this may result in duplicate notifications being sent: %s\n", err)
1171-
} else {
1172-
cliui.Info(inv.Stdout, "Gracefully shut down notifications manager\n")
1173-
}
1153+
// Stop the notification manager, which will cause any buffered updates to the store to be flushed.
1154+
// If the Stop() call times out, messages that were sent but not reflected as such in the store will have
1155+
// their leases expire after a period of time and will be re-queued for sending.
1156+
// See CODER_NOTIFICATIONS_LEASE_PERIOD.
1157+
cliui.Info(inv.Stdout, "Shutting down notifications manager..."+"\n")
1158+
err = shutdownWithTimeout(notificationsManager.Stop, 5*time.Second)
1159+
if err != nil {
1160+
cliui.Warnf(inv.Stderr, "Notifications manager shutdown took longer than 5s, "+
1161+
"this may result in duplicate notifications being sent: %s\n", err)
1162+
} else {
1163+
cliui.Info(inv.Stdout, "Gracefully shut down notifications manager\n")
11741164
}
11751165

11761166
// Shut down provisioners before waiting for WebSockets

‎coderd/coderd.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,12 @@ import (
3737
"tailscale.com/util/singleflight"
3838

3939
"cdr.dev/slog"
40+
"github.com/coder/quartz"
41+
"github.com/coder/serpent"
42+
4043
"github.com/coder/coder/v2/coderd/entitlements"
4144
"github.com/coder/coder/v2/coderd/idpsync"
4245
"github.com/coder/coder/v2/coderd/runtimeconfig"
43-
"github.com/coder/quartz"
44-
"github.com/coder/serpent"
4546

4647
agentproto "github.com/coder/coder/v2/agent/proto"
4748
"github.com/coder/coder/v2/buildinfo"
@@ -1257,10 +1258,7 @@ func New(options *Options) *API {
12571258
})
12581259
})
12591260
r.Route("/notifications", func(r chi.Router) {
1260-
r.Use(
1261-
apiKeyMiddleware,
1262-
httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentNotifications),
1263-
)
1261+
r.Use(apiKeyMiddleware)
12641262
r.Get("/settings", api.notificationsSettings)
12651263
r.Put("/settings", api.putNotificationsSettings)
12661264
r.Route("/templates", func(r chi.Router) {

‎coderd/notifications/manager.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ type Manager struct {
5454

5555
runOnce sync.Once
5656
stopOnce sync.Once
57+
doneOnce sync.Once
5758
stop chan any
5859
done chan any
5960

@@ -153,7 +154,9 @@ func (m *Manager) Run(ctx context.Context) {
153154
// events, creating a notifier, and publishing bulk dispatch result updates to the store.
154155
func (m *Manager) loop(ctx context.Context) error {
155156
defer func() {
156-
close(m.done)
157+
m.doneOnce.Do(func() {
158+
close(m.done)
159+
})
157160
m.log.Info(context.Background(), "notification manager stopped")
158161
}()
159162

@@ -364,7 +367,9 @@ func (m *Manager) Stop(ctx context.Context) error {
364367
// If the notifier hasn't been started, we don't need to wait for anything.
365368
// This is only really during testing when we want to enqueue messages only but not deliver them.
366369
if m.notifier == nil {
367-
close(m.done)
370+
m.doneOnce.Do(func() {
371+
close(m.done)
372+
})
368373
} else {
369374
m.notifier.stop()
370375
}

‎coderd/notifications/notifications_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1187,7 +1187,6 @@ func createOpts(t *testing.T) *coderdtest.Options {
11871187
t.Helper()
11881188

11891189
dt := coderdtest.DeploymentValues(t)
1190-
dt.Experiments = []string{string(codersdk.ExperimentNotifications)}
11911190
return &coderdtest.Options{
11921191
DeploymentValues: dt,
11931192
}

‎coderd/notifications/reports/generator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func NewReportGenerator(ctx context.Context, logger slog.Logger, db database.Sto
4949
return nil
5050
}
5151

52-
err = reportFailedWorkspaceBuilds(ctx, logger, db, enqueuer, clk)
52+
err = reportFailedWorkspaceBuilds(ctx, logger, tx, enqueuer, clk)
5353
if err != nil {
5454
return xerrors.Errorf("unable to generate reports with failed workspace builds: %w", err)
5555
}

‎coderd/notifications_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ func createOpts(t *testing.T) *coderdtest.Options {
2020
t.Helper()
2121

2222
dt := coderdtest.DeploymentValues(t)
23-
dt.Experiments = []string{string(codersdk.ExperimentNotifications)}
2423
return &coderdtest.Options{
2524
DeploymentValues: dt,
2625
}

‎codersdk/deployment.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2901,7 +2901,7 @@ const (
29012901
// users to opt-in to via --experimental='*'.
29022902
// Experiments that are not ready for consumption by all users should
29032903
// not be included here and will be essentially hidden.
2904-
var ExperimentsAll = Experiments{ExperimentNotifications}
2904+
var ExperimentsAll = Experiments{}
29052905

29062906
// Experiments is a list of experiments.
29072907
// Multiple experiments may be enabled at the same time.

‎enterprise/coderd/coderd.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,6 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
448448
// with the below route, we need to register this route without any mounts or groups to make both work.
449449
r.With(
450450
apiKeyMiddleware,
451-
httpmw.RequireExperiment(api.AGPL.Experiments, codersdk.ExperimentNotifications),
452451
httpmw.ExtractNotificationTemplateParam(options.Database),
453452
).Put("/notifications/templates/{notification_template}/method", api.updateNotificationTemplateMethod)
454453
})

‎enterprise/coderd/notifications_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ func createOpts(t *testing.T) *coderdenttest.Options {
2323
t.Helper()
2424

2525
dt := coderdtest.DeploymentValues(t)
26-
dt.Experiments = []string{string(codersdk.ExperimentNotifications)}
2726
return &coderdenttest.Options{
2827
Options: &coderdtest.Options{
2928
DeploymentValues: dt,

‎site/src/components/FeatureStageBadge/FeatureStageBadge.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { docs } from "utils/docs";
1010
* All types of feature that we are currently supporting. Defined as record to
1111
* ensure that we can't accidentally make typos when writing the badge text.
1212
*/
13-
const featureStageBadgeTypes = {
13+
export const featureStageBadgeTypes = {
1414
beta: "beta",
1515
experimental: "experimental",
1616
} as const satisfies Record<string, ReactNode>;

‎site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export const NotificationsPage: FC = () => {
4343
title="Notifications"
4444
description="Control delivery methods for notifications on this deployment."
4545
layout="fluid"
46+
featureStage={"beta"}
4647
>
4748
<Tabs active={tab}>
4849
<TabsList>

‎site/src/pages/DeploySettingsPage/Sidebar.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import NotificationsIcon from "@mui/icons-material/NotificationsNoneOutlined";
77
import Globe from "@mui/icons-material/PublicOutlined";
88
import ApprovalIcon from "@mui/icons-material/VerifiedUserOutlined";
99
import VpnKeyOutlined from "@mui/icons-material/VpnKeyOutlined";
10+
import { FeatureStageBadge } from "components/FeatureStageBadge/FeatureStageBadge";
1011
import { GitIcon } from "components/Icons/GitIcon";
1112
import {
1213
Sidebar as BaseSidebar,
@@ -51,11 +52,9 @@ export const Sidebar: FC = () => {
5152
<SidebarNavItem href="observability" icon={InsertChartIcon}>
5253
Observability
5354
</SidebarNavItem>
54-
{experiments.includes("notifications") && (
55-
<SidebarNavItem href="notifications" icon={NotificationsIcon}>
56-
Notifications
57-
</SidebarNavItem>
58-
)}
55+
<SidebarNavItem href="notifications" icon={NotificationsIcon}>
56+
Notifications <FeatureStageBadge contentType="beta" size="sm" />
57+
</SidebarNavItem>
5958
</BaseSidebar>
6059
);
6160
};

‎site/src/pages/ManagementSettingsPage/SidebarView.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,12 @@ const DeploymentSettingsNavigation: FC<DeploymentSettingsNavigationProps> = ({
148148
Users
149149
</SidebarNavSubItem>
150150
)}
151-
{experiments.includes("notifications") && (
151+
<Stack direction="row" alignItems="center" css={{ gap: 0 }}>
152152
<SidebarNavSubItem href="notifications">
153153
Notifications
154154
</SidebarNavSubItem>
155-
)}
155+
<FeatureStageBadge contentType="beta" size="sm" />
156+
</Stack>
156157
</Stack>
157158
)}
158159
</div>

‎site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ export const NotificationsPage: FC = () => {
9999
title="Notifications"
100100
description="Configure your notification preferences. Icons on the right of each notification indicate delivery method, either SMTP or Webhook."
101101
layout="fluid"
102+
featureStage="beta"
102103
>
103104
{ready ? (
104105
<Stack spacing={4}>

‎site/src/pages/UserSettingsPage/Section.tsx

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
import type { Interpolation, Theme } from "@emotion/react";
2+
import {
3+
FeatureStageBadge,
4+
type featureStageBadgeTypes,
5+
} from "components/FeatureStageBadge/FeatureStageBadge";
6+
import { Stack } from "components/Stack/Stack";
27
import type { FC, ReactNode } from "react";
38

49
type SectionLayout = "fixed" | "fluid";
@@ -13,6 +18,7 @@ export interface SectionProps {
1318
layout?: SectionLayout;
1419
className?: string;
1520
children?: ReactNode;
21+
featureStage?: keyof typeof featureStageBadgeTypes;
1622
}
1723

1824
export const Section: FC<SectionProps> = ({
@@ -24,6 +30,7 @@ export const Section: FC<SectionProps> = ({
2430
className = "",
2531
children,
2632
layout = "fixed",
33+
featureStage,
2734
}) => {
2835
return (
2936
<section className={className} id={id} data-testid={id}>
@@ -32,16 +39,25 @@ export const Section: FC<SectionProps> = ({
3239
<div css={styles.header}>
3340
<div>
3441
{title && (
35-
<h4
36-
css={{
37-
fontSize: 24,
38-
fontWeight: 500,
39-
margin: 0,
40-
marginBottom: 8,
41-
}}
42-
>
43-
{title}
44-
</h4>
42+
<Stack direction={"row"} alignItems="center">
43+
<h4
44+
css={{
45+
fontSize: 24,
46+
fontWeight: 500,
47+
margin: 0,
48+
marginBottom: 8,
49+
}}
50+
>
51+
{title}
52+
</h4>
53+
{featureStage && (
54+
<FeatureStageBadge
55+
contentType={featureStage}
56+
size="lg"
57+
css={{ marginBottom: "5px" }}
58+
/>
59+
)}
60+
</Stack>
4561
)}
4662
{description && typeof description === "string" && (
4763
<p css={styles.description}>{description}</p>

‎site/src/pages/UserSettingsPage/Sidebar.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import NotificationsIcon from "@mui/icons-material/NotificationsNoneOutlined";
66
import AccountIcon from "@mui/icons-material/Person";
77
import VpnKeyOutlined from "@mui/icons-material/VpnKeyOutlined";
88
import type { User } from "api/typesGenerated";
9+
import { FeatureStageBadge } from "components/FeatureStageBadge/FeatureStageBadge";
910
import { GitIcon } from "components/Icons/GitIcon";
1011
import {
1112
Sidebar as BaseSidebar,
@@ -57,11 +58,9 @@ export const Sidebar: FC<SidebarProps> = ({ user }) => {
5758
<SidebarNavItem href="tokens" icon={VpnKeyOutlined}>
5859
Tokens
5960
</SidebarNavItem>
60-
{experiments.includes("notifications") && (
61-
<SidebarNavItem href="notifications" icon={NotificationsIcon}>
62-
Notifications
63-
</SidebarNavItem>
64-
)}
61+
<SidebarNavItem href="notifications" icon={NotificationsIcon}>
62+
Notifications <FeatureStageBadge contentType="beta" size="sm" />
63+
</SidebarNavItem>
6564
</BaseSidebar>
6665
);
6766
};

0 commit comments

Comments
 (0)