From b6237474d245f8c633752ca809732aef87fb3944 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 6 Jan 2023 12:26:14 +0200 Subject: [PATCH] Fix README and basic startup example (#3275) Follow-up of https://github.com/neondatabase/neon/pull/3270 which made an example from main README.md not working. Fixes that, by adding a way to specify a default tenant now and modifies the basic neon_local test to start postgres and check branching. Not all neon_local commands are implemented, so not all README.md contents is tested yet. --- README.md | 11 ++- control_plane/src/bin/neon_local.rs | 20 ++++- control_plane/src/local_env.rs | 5 -- test_runner/fixtures/neon_fixtures.py | 100 ++++++++------------- test_runner/regress/test_neon_local_cli.py | 13 ++- 5 files changed, 72 insertions(+), 77 deletions(-) diff --git a/README.md b/README.md index fa5c1626e4..7b629e71a5 100644 --- a/README.md +++ b/README.md @@ -118,11 +118,8 @@ Python (3.9 or higher), and install python3 packages using `./scripts/pysync` (r # Later that would be responsibility of a package install script > ./target/debug/neon_local init Starting pageserver at '127.0.0.1:64000' in '.neon'. -pageserver started, pid: 2545906 -Successfully initialized timeline de200bd42b49cc1814412c7e592dd6e9 -Stopped pageserver 1 process with pid 2545906 -# start pageserver and safekeeper +# start pageserver, safekeeper, and broker for their intercommunication > ./target/debug/neon_local start Starting neon broker at 127.0.0.1:50051 storage_broker started, pid: 2918372 @@ -131,6 +128,12 @@ pageserver started, pid: 2918386 Starting safekeeper at '127.0.0.1:5454' in '.neon/safekeepers/sk1'. safekeeper 1 started, pid: 2918437 +# create initial tenant and use it as a default for every future neon_local invocation +> ./target/debug/neon_local tenant create --set-default +tenant 9ef87a5bf0d92544f6fafeeb3239695c successfully created on the pageserver +Created an initial timeline 'de200bd42b49cc1814412c7e592dd6e9' at Lsn 0/16B5A50 for tenant: 9ef87a5bf0d92544f6fafeeb3239695c +Setting tenant 9ef87a5bf0d92544f6fafeeb3239695c as a default one + # start postgres compute node > ./target/debug/neon_local pg start main Starting new postgres (v14) main on timeline de200bd42b49cc1814412c7e592dd6e9 ... diff --git a/control_plane/src/bin/neon_local.rs b/control_plane/src/bin/neon_local.rs index e4d0680c9e..4b2aa3c957 100644 --- a/control_plane/src/bin/neon_local.rs +++ b/control_plane/src/bin/neon_local.rs @@ -263,7 +263,7 @@ fn get_tenant_id(sub_match: &ArgMatches, env: &local_env::LocalEnv) -> anyhow::R } else if let Some(default_id) = env.default_tenant_id { Ok(default_id) } else { - bail!("No tenant id. Use --tenant-id, or set 'default_tenant_id' in the config file"); + anyhow::bail!("No tenant id. Use --tenant-id, or set a default tenant"); } } @@ -372,6 +372,17 @@ fn handle_tenant(tenant_match: &ArgMatches, env: &mut local_env::LocalEnv) -> an println!( "Created an initial timeline '{new_timeline_id}' at Lsn {last_record_lsn} for tenant: {new_tenant_id}", ); + + if create_match.get_flag("set-default") { + println!("Setting tenant {new_tenant_id} as a default one"); + env.default_tenant_id = Some(new_tenant_id); + } + } + Some(("set-default", set_default_match)) => { + let tenant_id = + parse_tenant_id(set_default_match)?.context("No tenant id specified")?; + println!("Setting tenant {tenant_id} as a default one"); + env.default_tenant_id = Some(tenant_id); } Some(("config", create_match)) => { let tenant_id = get_tenant_id(create_match, env)?; @@ -975,11 +986,14 @@ fn cli() -> Command { .arg(timeline_id_arg.clone().help("Use a specific timeline id when creating a tenant and its initial timeline")) .arg(Arg::new("config").short('c').num_args(1).action(ArgAction::Append).required(false)) .arg(pg_version_arg.clone()) + .arg(Arg::new("set-default").long("set-default").action(ArgAction::SetTrue).required(false) + .help("Use this tenant in future CLI commands where tenant_id is needed, but not specified")) ) + .subcommand(Command::new("set-default").arg(tenant_id_arg.clone().required(true)) + .about("Set a particular tenant as default in future CLI commands where tenant_id is needed, but not specified")) .subcommand(Command::new("config") .arg(tenant_id_arg.clone()) - .arg(Arg::new("config").short('c').num_args(1).action(ArgAction::Append).required(false)) - ) + .arg(Arg::new("config").short('c').num_args(1).action(ArgAction::Append).required(false))) ) .subcommand( Command::new("pageserver") diff --git a/control_plane/src/local_env.rs b/control_plane/src/local_env.rs index ea936640ec..003152c578 100644 --- a/control_plane/src/local_env.rs +++ b/control_plane/src/local_env.rs @@ -296,11 +296,6 @@ impl LocalEnv { env.neon_distrib_dir = env::current_exe()?.parent().unwrap().to_owned(); } - // If no initial tenant ID was given, generate it. - if env.default_tenant_id.is_none() { - env.default_tenant_id = Some(TenantId::generate()); - } - env.base_data_dir = base_path(); Ok(env) diff --git a/test_runner/fixtures/neon_fixtures.py b/test_runner/fixtures/neon_fixtures.py index 97bc694543..bdd3dc004e 100644 --- a/test_runner/fixtures/neon_fixtures.py +++ b/test_runner/fixtures/neon_fixtures.py @@ -18,6 +18,7 @@ from contextlib import closing, contextmanager from dataclasses import dataclass, field from enum import Flag, auto from functools import cached_property +from itertools import chain, product from pathlib import Path from types import TracebackType from typing import Any, Dict, Iterator, List, Optional, Tuple, Type, Union, cast @@ -1567,6 +1568,7 @@ class NeonCli(AbstractNeonCli): tenant_id: Optional[TenantId] = None, timeline_id: Optional[TimelineId] = None, conf: Optional[Dict[str, str]] = None, + set_default: bool = False, ) -> Tuple[TenantId, TimelineId]: """ Creates a new tenant, returns its id and its initial timeline's id. @@ -1575,47 +1577,51 @@ class NeonCli(AbstractNeonCli): tenant_id = TenantId.generate() if timeline_id is None: timeline_id = TimelineId.generate() - if conf is None: - res = self.raw_cli( - [ - "tenant", - "create", - "--tenant-id", - str(tenant_id), - "--timeline-id", - str(timeline_id), - "--pg-version", - self.env.pg_version, - ] - ) - else: - res = self.raw_cli( - [ - "tenant", - "create", - "--tenant-id", - str(tenant_id), - "--timeline-id", - str(timeline_id), - "--pg-version", - self.env.pg_version, - ] - + sum(list(map(lambda kv: (["-c", kv[0] + ":" + kv[1]]), conf.items())), []) + + args = [ + "tenant", + "create", + "--tenant-id", + str(tenant_id), + "--timeline-id", + str(timeline_id), + "--pg-version", + self.env.pg_version, + ] + if conf is not None: + args.extend( + chain.from_iterable( + product(["-c"], (f"{key}:{value}" for key, value in conf.items())) + ) ) + if set_default: + args.append("--set-default") + + res = self.raw_cli(args) res.check_returncode() return tenant_id, timeline_id + def set_default(self, tenant_id: TenantId): + """ + Update default tenant for future operations that require tenant_id. + """ + res = self.raw_cli(["tenant", "set-default", "--tenant-id", str(tenant_id)]) + res.check_returncode() + def config_tenant(self, tenant_id: TenantId, conf: Dict[str, str]): """ Update tenant config. """ - if conf is None: - res = self.raw_cli(["tenant", "config", "--tenant-id", str(tenant_id)]) - else: - res = self.raw_cli( - ["tenant", "config", "--tenant-id", str(tenant_id)] - + sum(list(map(lambda kv: (["-c", kv[0] + ":" + kv[1]]), conf.items())), []) + + args = ["tenant", "config", "--tenant-id", str(tenant_id)] + if conf is not None: + args.extend( + chain.from_iterable( + product(["-c"], (f"{key}:{value}" for key, value in conf.items())) + ) ) + + res = self.raw_cli(args) res.check_returncode() def list_tenants(self) -> "subprocess.CompletedProcess[str]": @@ -1650,36 +1656,6 @@ class NeonCli(AbstractNeonCli): return TimelineId(str(created_timeline_id)) - def create_root_branch( - self, - branch_name: str, - tenant_id: Optional[TenantId] = None, - ): - cmd = [ - "timeline", - "create", - "--branch-name", - branch_name, - "--tenant-id", - str(tenant_id or self.env.initial_tenant), - "--pg-version", - self.env.pg_version, - ] - - res = self.raw_cli(cmd) - res.check_returncode() - - matches = CREATE_TIMELINE_ID_EXTRACTOR.search(res.stdout) - - created_timeline_id = None - if matches is not None: - created_timeline_id = matches.group("timeline_id") - - if created_timeline_id is None: - raise Exception("could not find timeline id after `neon timeline create` invocation") - else: - return TimelineId(created_timeline_id) - def create_branch( self, new_branch_name: str = DEFAULT_BRANCH_NAME, diff --git a/test_runner/regress/test_neon_local_cli.py b/test_runner/regress/test_neon_local_cli.py index e8f01ccf55..49c063ce44 100644 --- a/test_runner/regress/test_neon_local_cli.py +++ b/test_runner/regress/test_neon_local_cli.py @@ -2,9 +2,16 @@ from fixtures.neon_fixtures import NeonEnvBuilder # Test that neon cli is able to start and stop all processes with the user defaults. -# def test_neon_cli_basics(neon_simple_env: NeonEnv): +# Repeats the example from README.md as close as it can def test_neon_cli_basics(neon_env_builder: NeonEnvBuilder): env = neon_env_builder.init_configs() + # Skipping the init step that creates a local tenant in Pytest tests + try: + env.neon_cli.start() + env.neon_cli.create_tenant(tenant_id=env.initial_tenant, set_default=True) + env.neon_cli.pg_start(node_name="main") - env.neon_cli.start() - env.neon_cli.stop() + env.neon_cli.create_branch(new_branch_name="migration_check") + env.neon_cli.pg_start(node_name="migration_check") + finally: + env.neon_cli.stop()