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.
This commit is contained in:
Danny Kopping
2024-10-01 15:43:47 +02:00
committed by GitHub
parent edb4485afd
commit 11f7b1b3f5
17 changed files with 95 additions and 90 deletions

View File

@ -20,7 +20,6 @@ func createOpts(t *testing.T) *coderdtest.Options {
t.Helper()
dt := coderdtest.DeploymentValues(t)
dt.Experiments = []string{string(codersdk.ExperimentNotifications)}
return &coderdtest.Options{
DeploymentValues: dt,
}

View File

@ -56,15 +56,16 @@ import (
"cdr.dev/slog"
"cdr.dev/slog/sloggers/sloghuman"
"github.com/coder/coder/v2/coderd/entitlements"
"github.com/coder/coder/v2/coderd/notifications/reports"
"github.com/coder/coder/v2/coderd/runtimeconfig"
"github.com/coder/pretty"
"github.com/coder/quartz"
"github.com/coder/retry"
"github.com/coder/serpent"
"github.com/coder/wgtunnel/tunnelsdk"
"github.com/coder/coder/v2/coderd/entitlements"
"github.com/coder/coder/v2/coderd/notifications/reports"
"github.com/coder/coder/v2/coderd/runtimeconfig"
"github.com/coder/coder/v2/buildinfo"
"github.com/coder/coder/v2/cli/clilog"
"github.com/coder/coder/v2/cli/cliui"
@ -684,10 +685,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
options.OIDCConfig = oc
}
experiments := coderd.ReadExperiments(
options.Logger, options.DeploymentValues.Experiments.Value(),
)
// We'll read from this channel in the select below that tracks shutdown. If it remains
// nil, that case of the select will just never fire, but it's important not to have a
// "bare" read on this channel.
@ -951,6 +948,33 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
return xerrors.Errorf("write config url: %w", err)
}
// Manage notifications.
cfg := options.DeploymentValues.Notifications
metrics := notifications.NewMetrics(options.PrometheusRegistry)
helpers := templateHelpers(options)
// The enqueuer is responsible for enqueueing notifications to the given store.
enqueuer, err := notifications.NewStoreEnqueuer(cfg, options.Database, helpers, logger.Named("notifications.enqueuer"), quartz.NewReal())
if err != nil {
return xerrors.Errorf("failed to instantiate notification store enqueuer: %w", err)
}
options.NotificationsEnqueuer = enqueuer
// The notification manager is responsible for:
// - creating notifiers and managing their lifecycles (notifiers are responsible for dequeueing/sending notifications)
// - keeping the store updated with status updates
notificationsManager, err := notifications.NewManager(cfg, options.Database, helpers, metrics, logger.Named("notifications.manager"))
if err != nil {
return xerrors.Errorf("failed to instantiate notification manager: %w", err)
}
// nolint:gocritic // TODO: create own role.
notificationsManager.Run(dbauthz.AsSystemRestricted(ctx))
// Run report generator to distribute periodic reports.
notificationReportGenerator := reports.NewReportGenerator(ctx, logger.Named("notifications.report_generator"), options.Database, options.NotificationsEnqueuer, quartz.NewReal())
defer notificationReportGenerator.Close()
// Since errCh only has one buffered slot, all routines
// sending on it must be wrapped in a select/default to
// avoid leaving dangling goroutines waiting for the
@ -1007,38 +1031,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
options.WorkspaceUsageTracker = tracker
defer tracker.Close()
// Manage notifications.
var (
notificationsManager *notifications.Manager
)
if experiments.Enabled(codersdk.ExperimentNotifications) {
cfg := options.DeploymentValues.Notifications
metrics := notifications.NewMetrics(options.PrometheusRegistry)
helpers := templateHelpers(options)
// The enqueuer is responsible for enqueueing notifications to the given store.
enqueuer, err := notifications.NewStoreEnqueuer(cfg, options.Database, helpers, logger.Named("notifications.enqueuer"), quartz.NewReal())
if err != nil {
return xerrors.Errorf("failed to instantiate notification store enqueuer: %w", err)
}
options.NotificationsEnqueuer = enqueuer
// The notification manager is responsible for:
// - creating notifiers and managing their lifecycles (notifiers are responsible for dequeueing/sending notifications)
// - keeping the store updated with status updates
notificationsManager, err = notifications.NewManager(cfg, options.Database, helpers, metrics, logger.Named("notifications.manager"))
if err != nil {
return xerrors.Errorf("failed to instantiate notification manager: %w", err)
}
// nolint:gocritic // TODO: create own role.
notificationsManager.Run(dbauthz.AsSystemRestricted(ctx))
// Run report generator to distribute periodic reports.
notificationReportGenerator := reports.NewReportGenerator(ctx, logger.Named("notifications.report_generator"), options.Database, options.NotificationsEnqueuer, quartz.NewReal())
defer notificationReportGenerator.Close()
}
// Wrap the server in middleware that redirects to the access URL if
// the request is not to a local IP.
var handler http.Handler = coderAPI.RootHandler
@ -1158,19 +1150,17 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
// Cancel any remaining in-flight requests.
shutdownConns()
if notificationsManager != nil {
// Stop the notification manager, which will cause any buffered updates to the store to be flushed.
// If the Stop() call times out, messages that were sent but not reflected as such in the store will have
// their leases expire after a period of time and will be re-queued for sending.
// See CODER_NOTIFICATIONS_LEASE_PERIOD.
cliui.Info(inv.Stdout, "Shutting down notifications manager..."+"\n")
err = shutdownWithTimeout(notificationsManager.Stop, 5*time.Second)
if err != nil {
cliui.Warnf(inv.Stderr, "Notifications manager shutdown took longer than 5s, "+
"this may result in duplicate notifications being sent: %s\n", err)
} else {
cliui.Info(inv.Stdout, "Gracefully shut down notifications manager\n")
}
// Stop the notification manager, which will cause any buffered updates to the store to be flushed.
// If the Stop() call times out, messages that were sent but not reflected as such in the store will have
// their leases expire after a period of time and will be re-queued for sending.
// See CODER_NOTIFICATIONS_LEASE_PERIOD.
cliui.Info(inv.Stdout, "Shutting down notifications manager..."+"\n")
err = shutdownWithTimeout(notificationsManager.Stop, 5*time.Second)
if err != nil {
cliui.Warnf(inv.Stderr, "Notifications manager shutdown took longer than 5s, "+
"this may result in duplicate notifications being sent: %s\n", err)
} else {
cliui.Info(inv.Stdout, "Gracefully shut down notifications manager\n")
}
// Shut down provisioners before waiting for WebSockets

View File

@ -37,11 +37,12 @@ import (
"tailscale.com/util/singleflight"
"cdr.dev/slog"
"github.com/coder/quartz"
"github.com/coder/serpent"
"github.com/coder/coder/v2/coderd/entitlements"
"github.com/coder/coder/v2/coderd/idpsync"
"github.com/coder/coder/v2/coderd/runtimeconfig"
"github.com/coder/quartz"
"github.com/coder/serpent"
agentproto "github.com/coder/coder/v2/agent/proto"
"github.com/coder/coder/v2/buildinfo"
@ -1257,10 +1258,7 @@ func New(options *Options) *API {
})
})
r.Route("/notifications", func(r chi.Router) {
r.Use(
apiKeyMiddleware,
httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentNotifications),
)
r.Use(apiKeyMiddleware)
r.Get("/settings", api.notificationsSettings)
r.Put("/settings", api.putNotificationsSettings)
r.Route("/templates", func(r chi.Router) {

View File

@ -54,6 +54,7 @@ type Manager struct {
runOnce sync.Once
stopOnce sync.Once
doneOnce sync.Once
stop chan any
done chan any
@ -153,7 +154,9 @@ func (m *Manager) Run(ctx context.Context) {
// events, creating a notifier, and publishing bulk dispatch result updates to the store.
func (m *Manager) loop(ctx context.Context) error {
defer func() {
close(m.done)
m.doneOnce.Do(func() {
close(m.done)
})
m.log.Info(context.Background(), "notification manager stopped")
}()
@ -364,7 +367,9 @@ func (m *Manager) Stop(ctx context.Context) error {
// If the notifier hasn't been started, we don't need to wait for anything.
// This is only really during testing when we want to enqueue messages only but not deliver them.
if m.notifier == nil {
close(m.done)
m.doneOnce.Do(func() {
close(m.done)
})
} else {
m.notifier.stop()
}

View File

@ -1187,7 +1187,6 @@ func createOpts(t *testing.T) *coderdtest.Options {
t.Helper()
dt := coderdtest.DeploymentValues(t)
dt.Experiments = []string{string(codersdk.ExperimentNotifications)}
return &coderdtest.Options{
DeploymentValues: dt,
}

View File

@ -49,7 +49,7 @@ func NewReportGenerator(ctx context.Context, logger slog.Logger, db database.Sto
return nil
}
err = reportFailedWorkspaceBuilds(ctx, logger, db, enqueuer, clk)
err = reportFailedWorkspaceBuilds(ctx, logger, tx, enqueuer, clk)
if err != nil {
return xerrors.Errorf("unable to generate reports with failed workspace builds: %w", err)
}

View File

@ -20,7 +20,6 @@ func createOpts(t *testing.T) *coderdtest.Options {
t.Helper()
dt := coderdtest.DeploymentValues(t)
dt.Experiments = []string{string(codersdk.ExperimentNotifications)}
return &coderdtest.Options{
DeploymentValues: dt,
}

View File

@ -2901,7 +2901,7 @@ const (
// users to opt-in to via --experimental='*'.
// Experiments that are not ready for consumption by all users should
// not be included here and will be essentially hidden.
var ExperimentsAll = Experiments{ExperimentNotifications}
var ExperimentsAll = Experiments{}
// Experiments is a list of experiments.
// Multiple experiments may be enabled at the same time.

View File

@ -448,7 +448,6 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
// with the below route, we need to register this route without any mounts or groups to make both work.
r.With(
apiKeyMiddleware,
httpmw.RequireExperiment(api.AGPL.Experiments, codersdk.ExperimentNotifications),
httpmw.ExtractNotificationTemplateParam(options.Database),
).Put("/notifications/templates/{notification_template}/method", api.updateNotificationTemplateMethod)
})

View File

@ -23,7 +23,6 @@ func createOpts(t *testing.T) *coderdenttest.Options {
t.Helper()
dt := coderdtest.DeploymentValues(t)
dt.Experiments = []string{string(codersdk.ExperimentNotifications)}
return &coderdenttest.Options{
Options: &coderdtest.Options{
DeploymentValues: dt,

View File

@ -10,7 +10,7 @@ import { docs } from "utils/docs";
* All types of feature that we are currently supporting. Defined as record to
* ensure that we can't accidentally make typos when writing the badge text.
*/
const featureStageBadgeTypes = {
export const featureStageBadgeTypes = {
beta: "beta",
experimental: "experimental",
} as const satisfies Record<string, ReactNode>;

View File

@ -43,6 +43,7 @@ export const NotificationsPage: FC = () => {
title="Notifications"
description="Control delivery methods for notifications on this deployment."
layout="fluid"
featureStage={"beta"}
>
<Tabs active={tab}>
<TabsList>

View File

@ -7,6 +7,7 @@ import NotificationsIcon from "@mui/icons-material/NotificationsNoneOutlined";
import Globe from "@mui/icons-material/PublicOutlined";
import ApprovalIcon from "@mui/icons-material/VerifiedUserOutlined";
import VpnKeyOutlined from "@mui/icons-material/VpnKeyOutlined";
import { FeatureStageBadge } from "components/FeatureStageBadge/FeatureStageBadge";
import { GitIcon } from "components/Icons/GitIcon";
import {
Sidebar as BaseSidebar,
@ -51,11 +52,9 @@ export const Sidebar: FC = () => {
<SidebarNavItem href="observability" icon={InsertChartIcon}>
Observability
</SidebarNavItem>
{experiments.includes("notifications") && (
<SidebarNavItem href="notifications" icon={NotificationsIcon}>
Notifications
</SidebarNavItem>
)}
<SidebarNavItem href="notifications" icon={NotificationsIcon}>
Notifications <FeatureStageBadge contentType="beta" size="sm" />
</SidebarNavItem>
</BaseSidebar>
);
};

View File

@ -148,11 +148,12 @@ const DeploymentSettingsNavigation: FC<DeploymentSettingsNavigationProps> = ({
Users
</SidebarNavSubItem>
)}
{experiments.includes("notifications") && (
<Stack direction="row" alignItems="center" css={{ gap: 0 }}>
<SidebarNavSubItem href="notifications">
Notifications
</SidebarNavSubItem>
)}
<FeatureStageBadge contentType="beta" size="sm" />
</Stack>
</Stack>
)}
</div>

View File

@ -99,6 +99,7 @@ export const NotificationsPage: FC = () => {
title="Notifications"
description="Configure your notification preferences. Icons on the right of each notification indicate delivery method, either SMTP or Webhook."
layout="fluid"
featureStage="beta"
>
{ready ? (
<Stack spacing={4}>

View File

@ -1,4 +1,9 @@
import type { Interpolation, Theme } from "@emotion/react";
import {
FeatureStageBadge,
type featureStageBadgeTypes,
} from "components/FeatureStageBadge/FeatureStageBadge";
import { Stack } from "components/Stack/Stack";
import type { FC, ReactNode } from "react";
type SectionLayout = "fixed" | "fluid";
@ -13,6 +18,7 @@ export interface SectionProps {
layout?: SectionLayout;
className?: string;
children?: ReactNode;
featureStage?: keyof typeof featureStageBadgeTypes;
}
export const Section: FC<SectionProps> = ({
@ -24,6 +30,7 @@ export const Section: FC<SectionProps> = ({
className = "",
children,
layout = "fixed",
featureStage,
}) => {
return (
<section className={className} id={id} data-testid={id}>
@ -32,16 +39,25 @@ export const Section: FC<SectionProps> = ({
<div css={styles.header}>
<div>
{title && (
<h4
css={{
fontSize: 24,
fontWeight: 500,
margin: 0,
marginBottom: 8,
}}
>
{title}
</h4>
<Stack direction={"row"} alignItems="center">
<h4
css={{
fontSize: 24,
fontWeight: 500,
margin: 0,
marginBottom: 8,
}}
>
{title}
</h4>
{featureStage && (
<FeatureStageBadge
contentType={featureStage}
size="lg"
css={{ marginBottom: "5px" }}
/>
)}
</Stack>
)}
{description && typeof description === "string" && (
<p css={styles.description}>{description}</p>

View File

@ -6,6 +6,7 @@ import NotificationsIcon from "@mui/icons-material/NotificationsNoneOutlined";
import AccountIcon from "@mui/icons-material/Person";
import VpnKeyOutlined from "@mui/icons-material/VpnKeyOutlined";
import type { User } from "api/typesGenerated";
import { FeatureStageBadge } from "components/FeatureStageBadge/FeatureStageBadge";
import { GitIcon } from "components/Icons/GitIcon";
import {
Sidebar as BaseSidebar,
@ -57,11 +58,9 @@ export const Sidebar: FC<SidebarProps> = ({ user }) => {
<SidebarNavItem href="tokens" icon={VpnKeyOutlined}>
Tokens
</SidebarNavItem>
{experiments.includes("notifications") && (
<SidebarNavItem href="notifications" icon={NotificationsIcon}>
Notifications
</SidebarNavItem>
)}
<SidebarNavItem href="notifications" icon={NotificationsIcon}>
Notifications <FeatureStageBadge contentType="beta" size="sm" />
</SidebarNavItem>
</BaseSidebar>
);
};