Skip to content

Commit c975bfb

Browse files
craig[bot]soma00333alyshanjahani-crl
committed
147978: dev: remove stale --oss flag from ui subcommand r=rickystewart a=soma00333 ### Overview This PR removes the deprecated `--oss` flag and its associated logic from the `dev ui` subcommand, resolving a piece of technical debt and simplifying the UI development workflow. Fixes #136606 ### Description of Changes The `dev ui watch` command and its helpers contained a boolean `--oss` flag and corresponding logic to build only the open-source parts of the UI. This build path is no longer used, as development now exclusively targets the CCL version. This change fully removes the `--oss` flag and its related logic from `pkg/cmd/dev/ui.go`: - The `ossFlag` constant has been deleted. - The `--oss` flag definition and its parsing logic have been removed from the `makeUIWatchCmd` function. - The build process for `dev ui watch` is now streamlined to always build the CCL version, removing all conditional build arguments. - As part of the cleanup, the `arrangeFilesForWatchers` helper function was refactored to remove the now-redundant `ossOnly` boolean parameter. This simplified its signature and call sites in the `test` and `storybook` subcommands as well. ### Verification Steps The changes were thoroughly verified locally to ensure correctness and prevent regressions. 1. **Environment Setup:** * The local `dev` tool cache (`bin/dev-versions/dev.110`) was manually cleared to ensure the changes to `ui.go` were correctly compiled and loaded. 2. **Command-Line Verification:** * A local DB cluster was started using `./cockroach start-single-node --insecure`. * The following `./dev ui` commands were executed sequentially, and their outputs were confirmed to match expectations: 1. `./dev ui watch --oss` * **Result:** Correctly failed with `ERROR: unknown flag: --oss`. 2. `./dev ui watch --db=http://localhost:8080` * **Result:** Successfully launched the DB Console (CCL version), which was accessible and fully functional at `http://localhost:3000`. 3. `./dev ui test` * **Result:** All UI unit tests (`db-console` and `cluster-ui`) passed successfully. 4. `./dev ui storybook --project db-console` * **Result:** Successfully launched Storybook, which was accessible and functional at `http://localhost:6006`. All verification steps passed, confirming that the `--oss` flag has been removed cleanly and that the associated refactoring did not introduce any side effects. 148112: roachtest: Add db-console/mixed-version-endpoints roachtest r=alyshanjahani-crl a=alyshanjahani-crl This commit registers a mixed-version variation of the db-console/endpoints roachtest. Part of: #145536 Release note: None Co-authored-by: soma00333 <[email protected]> Co-authored-by: Alyshan Jahani <[email protected]>
3 parents 5ccd93e + 94ba8d3 + ecd20f3 commit c975bfb

File tree

5 files changed

+47
-52
lines changed

5 files changed

+47
-52
lines changed

dev

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ fi
88
set -euo pipefail
99

1010
# Bump this counter to force rebuilding `dev` on all machines.
11-
DEV_VERSION=110
11+
DEV_VERSION=111
1212

1313
THIS_DIR=$(cd "$(dirname "$0")" && pwd)
1414
BINARY_DIR=$THIS_DIR/bin/dev-versions

pkg/cmd/dev/testdata/datadriven/ui

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,6 @@ pnpm --dir crdb-checkout/pkg/ui/workspaces/cluster-ui run build:watch
1616
pnpm --dir crdb-checkout/pkg/ui/workspaces/cluster-ui run tsc:watch
1717
pnpm --dir crdb-checkout/pkg/ui/workspaces/db-console exec webpack-dev-server --config webpack.config.js --mode development --env.WEBPACK_SERVE --env.dist=ccl --env.target=http://localhost:8080 --port 3000
1818

19-
exec
20-
dev ui watch --oss
21-
----
22-
bazel info workspace --color=no
23-
pnpm --dir crdb-checkout/pkg/ui install
24-
bazel build //pkg/ui/workspaces/cluster-ui:cluster-ui-lib
25-
bazel info bazel-bin --color=no
26-
bazel info workspace --color=no
27-
cp sandbox/pkg/ui/workspaces/db-console/src/js/protos.js crdb-checkout/pkg/ui/workspaces/db-console/src/js/protos.js
28-
cp sandbox/pkg/ui/workspaces/db-console/src/js/protos.d.ts crdb-checkout/pkg/ui/workspaces/db-console/src/js/protos.d.ts
29-
rm -rf crdb-checkout/pkg/ui/workspaces/cluster-ui/dist
30-
cp -r sandbox/pkg/ui/workspaces/cluster-ui/dist crdb-checkout/pkg/ui/workspaces/cluster-ui/dist
31-
pnpm --dir crdb-checkout/pkg/ui/workspaces/cluster-ui run build:watch
32-
pnpm --dir crdb-checkout/pkg/ui/workspaces/cluster-ui run tsc:watch
33-
pnpm --dir crdb-checkout/pkg/ui/workspaces/db-console exec webpack-dev-server --config webpack.config.js --mode development --env.WEBPACK_SERVE --env.dist=oss --env.target=http://localhost:8080 --port 3000
34-
3519
exec
3620
dev ui watch --secure
3721
----

pkg/cmd/dev/ui.go

Lines changed: 10 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,6 @@ import (
1717
"github.com/spf13/cobra"
1818
)
1919

20-
const (
21-
// ossFlag is the name of the boolean long (GNU-style) flag that builds only
22-
// the open-source parts of the UI.
23-
ossFlag = "oss"
24-
)
25-
2620
// makeUICmd initializes the top-level 'ui' subcommand.
2721
func makeUICmd(d *dev) *cobra.Command {
2822
uiCmd := &cobra.Command{
@@ -288,11 +282,6 @@ Replaces 'make ui-watch'.`,
288282
return err
289283
}
290284

291-
isOss, err := cmd.Flags().GetBool(ossFlag)
292-
if err != nil {
293-
return err
294-
}
295-
296285
// Ensure node dependencies are up-to-date.
297286
err = d.exec.CommandContextInheritingStdStreams(
298287
ctx,
@@ -309,13 +298,11 @@ Replaces 'make ui-watch'.`,
309298
args := []string{
310299
"build",
311300
"//pkg/ui/workspaces/cluster-ui:cluster-ui-lib",
312-
}
313-
if !isOss {
314-
args = append(args, "//pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl-lib")
315-
args = append(args, "//pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client_files")
316-
args = append(args, "//pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client")
317-
args = append(args, "//pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl_files")
318-
args = append(args, "//pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl")
301+
"//pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl-lib",
302+
"//pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client_files",
303+
"//pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client",
304+
"//pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl_files",
305+
"//pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl",
319306
}
320307
logCommand("bazel", args...)
321308
err = d.exec.CommandContextInheritingStdStreams(ctx, "bazel", args...)
@@ -325,7 +312,7 @@ Replaces 'make ui-watch'.`,
325312
return err
326313
}
327314

328-
if err := arrangeFilesForWatchers(d, isOss); err != nil {
315+
if err := arrangeFilesForWatchers(d); err != nil {
329316
log.Fatalf("failed to arrange files for watchers: %v", err)
330317
return err
331318
}
@@ -382,13 +369,6 @@ Replaces 'make ui-watch'.`,
382369
}
383370
}
384371

385-
var webpackDist string
386-
if isOss {
387-
webpackDist = "oss"
388-
} else {
389-
webpackDist = "ccl"
390-
}
391-
392372
args = []string{
393373
"--dir",
394374
dirs.dbConsole,
@@ -399,7 +379,7 @@ Replaces 'make ui-watch'.`,
399379
// Polyfill WEBPACK_SERVE for webpack v4; it's set in webpack v5 via
400380
// `webpack serve`.
401381
"--env.WEBPACK_SERVE",
402-
"--env.dist=" + webpackDist,
382+
"--env.dist=ccl",
403383
"--env.target=" + dbTarget,
404384
"--port", port,
405385
}
@@ -425,7 +405,6 @@ Replaces 'make ui-watch'.`,
425405
watchCmd.Flags().Int16P(portFlag, "p", 3000, "port to serve UI on")
426406
watchCmd.Flags().String(dbTargetFlag, "http://localhost:8080", "url to proxy DB requests to")
427407
watchCmd.Flags().Bool(secureFlag, false, "serve via HTTPS")
428-
watchCmd.Flags().Bool(ossFlag, false, "build only the open-source parts of the UI")
429408
watchCmd.Flags().StringArray(
430409
clusterUiDestinationsFlag,
431410
[]string{},
@@ -482,7 +461,7 @@ func makeUIStorybookCmd(d *dev) *cobra.Command {
482461
return err
483462
}
484463

485-
if err := arrangeFilesForWatchers(d /* ossOnly */, false); err != nil {
464+
if err := arrangeFilesForWatchers(d); err != nil {
486465
log.Fatalf("failed to arrange files for watchers: %v", err)
487466
return err
488467
}
@@ -745,7 +724,7 @@ func makeUICleanCmd(d *dev) *cobra.Command {
745724
// mode) to be executed from directly within a pkg/ui/workspaces/... directory.
746725
//
747726
// See https://github.com/bazelbuild/rules_nodejs/issues/2028
748-
func arrangeFilesForWatchers(d *dev, ossOnly bool) error {
727+
func arrangeFilesForWatchers(d *dev) error {
749728
bazelBin, err := d.getBazelBin(d.cli.Context(), []string{})
750729
if err != nil {
751730
return err
@@ -773,10 +752,6 @@ func arrangeFilesForWatchers(d *dev, ossOnly bool) error {
773752
return err
774753
}
775754

776-
if ossOnly {
777-
continue
778-
}
779-
780755
cclDst := filepath.Join(dbConsoleCclDst, relPath)
781756
if err := d.os.CopyFile(filepath.Join(dbConsoleCclSrc, relPath), cclDst); err != nil {
782757
return err
@@ -845,7 +820,7 @@ Replaces 'make ui-test' and 'make ui-test-watch'.`,
845820
return err
846821
}
847822

848-
err = arrangeFilesForWatchers(d, false /* ossOnly */)
823+
err = arrangeFilesForWatchers(d)
849824
if err != nil {
850825
// nolint:errwrap
851826
return fmt.Errorf("unable to arrange files properly for watch-mode testing: %+v", err)

pkg/cmd/roachtest/tests/db_console_endpoints.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
_ "embed"
1111
"encoding/json"
1212
"io"
13+
"math/rand"
1314
"net/http"
1415
"strconv"
1516
"strings"
@@ -19,6 +20,8 @@ import (
1920
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
2021
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
2122
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil"
23+
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/mixedversion"
24+
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec"
2225
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
2326
"github.com/cockroachdb/cockroach/pkg/roachpb"
2427
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
@@ -90,6 +93,38 @@ func registerDBConsoleEndpoints(r registry.Registry) {
9093
})
9194
}
9295

96+
func registerDBConsoleEndpointsMixedVersion(r registry.Registry) {
97+
r.Add(registry.TestSpec{
98+
Name: "db-console/mixed-version-endpoints",
99+
Owner: registry.OwnerObservability,
100+
Cluster: r.MakeClusterSpec(5, spec.WorkloadNode()),
101+
CompatibleClouds: registry.AllClouds,
102+
Suites: registry.Suites(registry.MixedVersion, registry.Nightly),
103+
Randomized: true,
104+
Run: runDBConsoleMixedVersion,
105+
Timeout: 1 * time.Hour,
106+
})
107+
}
108+
109+
func runDBConsoleMixedVersion(ctx context.Context, t test.Test, c cluster.Cluster) {
110+
mvt := mixedversion.NewTest(ctx, t, t.L(), c,
111+
c.CRDBNodes(),
112+
// We test only upgrades from 23.2 in this test because it uses
113+
// the `workload init` command, which is only supported
114+
// reliably multi-tenant mode starting from that version.
115+
mixedversion.MinimumSupportedVersion("v23.2.0"),
116+
)
117+
118+
mvt.InMixedVersion("test db console endpoints", func(ctx context.Context, l *logger.Logger, rng *rand.Rand, h *mixedversion.Helper) error {
119+
if err := initializeSchemaAndIDs(ctx, c, t.L()); err != nil {
120+
t.Fatal(err)
121+
}
122+
return testEndpoints(ctx, c, l, getEndpoints(t), true)
123+
})
124+
125+
mvt.Run()
126+
}
127+
93128
func runDBConsole(ctx context.Context, t test.Test, c cluster.Cluster) {
94129
c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings())
95130

pkg/cmd/roachtest/tests/registry.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ func RegisterTests(r registry.Registry) {
176176
registerSqlStatsMixedVersion(r)
177177
registerDbConsoleCypress(r)
178178
registerDBConsoleEndpoints(r)
179+
registerDBConsoleEndpointsMixedVersion(r)
179180
registerTTLRestart(r)
180181
perturbation.RegisterTests(r)
181182
}

0 commit comments

Comments
 (0)