Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clp-package: Add log-viewer-webui as a component. #476

Merged
merged 14 commits into from
Jul 12, 2024
54 changes: 50 additions & 4 deletions Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ vars:
G_PACKAGE_VENV_DIR: "{{.G_BUILD_DIR}}/package-venv"
G_WEBUI_BUILD_DIR: "{{.G_BUILD_DIR}}/webui"
G_WEBUI_NODEJS_BUILD_DIR: "{{.G_BUILD_DIR}}/webui-nodejs"
G_LOG_VIEWER_WEBUI_BUILD_DIR: "{{.G_BUILD_DIR}}/log-viewer-webui"
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved
G_WEBUI_NODEJS_BIN_DIR: "{{.G_WEBUI_NODEJS_BUILD_DIR}}/bin"
G_LOG_VIEWER_WEBUI_BIN_DIR: "{{.G_NODEJS_22_DIR}}/bin"
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved

# Versions
G_PACKAGE_VERSION: "0.2.0-dev"
Expand Down Expand Up @@ -81,6 +83,7 @@ tasks:
DATA_DIR: "{{.OUTPUT_DIR}}"
- "webui"
- "webui-nodejs"
- "log-viewer-webui"
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved
cmds:
- "rm -rf '{{.OUTPUT_DIR}}'"
- "rsync -a components/package-template/src/ '{{.OUTPUT_DIR}}'"
Expand All @@ -101,16 +104,30 @@ tasks:
"{{.G_CORE_COMPONENT_BUILD_DIR}}/clp"
"{{.G_CORE_COMPONENT_BUILD_DIR}}/clp-s"
"{{.G_CORE_COMPONENT_BUILD_DIR}}/reducer-server"
"{{.G_WEBUI_NODEJS_BIN_DIR}}/node"
"{{.OUTPUT_DIR}}/bin/"
- >-
rsync -a
"{{.G_WEBUI_NODEJS_BIN_DIR}}/node"
"{{.OUTPUT_DIR}}/bin/node-14"
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved
- >-
rsync -a
"{{.G_LOG_VIEWER_WEBUI_BIN_DIR}}/node"
"{{.OUTPUT_DIR}}/bin/node-22"
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved
- "mkdir -p '{{.OUTPUT_DIR}}/var/www/'"
- >-
rsync -a
"{{.G_WEBUI_BUILD_DIR}}/"
"{{.OUTPUT_DIR}}/var/www/"
"{{.OUTPUT_DIR}}/var/www/webui/"
- |-
cd "{{.OUTPUT_DIR}}/var/www/programs/server"
cd "{{.OUTPUT_DIR}}/var/www/webui/programs/server"
PATH="{{.G_WEBUI_NODEJS_BIN_DIR}}":$PATH npm install
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved
- >-
rsync -a
"{{.G_LOG_VIEWER_WEBUI_BUILD_DIR}}/"
"{{.OUTPUT_DIR}}/var/www/log_viewer/"
- |-
cd "{{.OUTPUT_DIR}}/var/www/log_viewer/server"
PATH="{{.G_LOG_VIEWER_WEBUI_BIN_DIR}}":$PATH npm i
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add the --production flag to prevent installations of the devDependencies.
With that said, I didn't find the related description on the https://docs.npmjs.com/cli/v10/commands/npm-ci page. If the flag is npm install specific, we shall explore the --omit or --include options.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the specification, if NODE_ENV is set to production, both npm install and npm clean-install will not install dev dependencies (here).
We can set NODE_ENV in the task to achieve this, but it also make sense to use the omit flag explicitly. let me know what do you think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'm fine with both the NODE_ENV and "--production" approches. For consistency with our other Node.js commands, the NODE_ENV approach seems better.

# This command must be last
- task: "utils:compute-checksum"
vars:
Expand Down Expand Up @@ -174,6 +191,36 @@ tasks:
vars:
COMPONENT: "{{.TASK}}"

log-viewer-webui:
deps:
- "init"
- "log-viewer-webui-node-modules"
- task: "utils:validate-checksum"
vars:
CHECKSUM_FILE: "{{.CHECKSUM_FILE}}"
DATA_DIR: "{{.OUTPUT_DIR}}"
dir: "components/log-viewer-webui"
vars:
CHECKSUM_FILE: "{{.G_BUILD_DIR}}/{{.TASK}}.md5"
OUTPUT_DIR: "{{.G_LOG_VIEWER_WEBUI_BUILD_DIR}}"
cmds:
- "rm -rf '{{.OUTPUT_DIR}}'"
- "rsync -a client server {{.OUTPUT_DIR}}/"
- |-
cd "{{.OUTPUT_DIR}}/client"
PATH="{{.G_LOG_VIEWER_WEBUI_BIN_DIR}}":$PATH npm run build
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We should build the client first and copy only the generated assets to the clp-package.
  2. We should avoid copying the .env file in case in the future it may contain sensitive information for debugging purposes.

e.g., (not tested)

Suggested change
- "rsync -a client server {{.OUTPUT_DIR}}/"
- |-
cd "{{.OUTPUT_DIR}}/client"
PATH="{{.G_LOG_VIEWER_WEBUI_BIN_DIR}}":$PATH npm run build
- |-
cd client
PATH="{{.G_LOG_VIEWER_WEBUI_BIN_DIR}}":$PATH npm clean-install
PATH="{{.G_LOG_VIEWER_WEBUI_BIN_DIR}}":$PATH npm run build
rsync -a dist "{{.OUTPUT_DIR}}/client/"
- |-
cd server
rsync -a src package-lock.json package.json {{.OUTPUT_DIR}}/server/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my experiment, I have noticed that building the client first in the repo has cause pycharm and vscode to get confused on the newly generated files.

This step only copies the files to under build/ (instead of build/clp_package), so it does not violate "copy only the generated assets to the clp-package.", so I would still prefer my original approach. I can update it to not copy .env file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, sorry for misreading that. It makes sense to copy the sources to the clp/build directory first. We just need to avoid packaging the .env file then.

- task: "utils:compute-checksum"
vars:
DATA_DIR: "{{.OUTPUT_DIR}}"
OUTPUT_FILE: "{{.CHECKSUM_FILE}}"
sources:
- "{{.G_BUILD_DIR}}/log-viewer-modules.md5"
- "{{.TASKFILE}}"
- "*"
- "client/**/*"
- "server/**/*"
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved
generates: ["{{.CHECKSUM_FILE}}"]

webui:
deps:
- "init"
Expand Down Expand Up @@ -306,7 +353,6 @@ tasks:
# tasks which depend on this task to only have to check one checksum file, we concatenate the
# three checksum files into one.
log-viewer-webui-node-modules:
internal: true
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved
vars:
# Checksum files
CHECKSUM_FILE: "{{.G_BUILD_DIR}}/{{.TASK}}.md5"
Expand Down
14 changes: 14 additions & 0 deletions components/clp-package-utils/clp_package_utils/general.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
CLP_DEFAULT_CREDENTIALS_FILE_PATH,
CLPConfig,
DB_COMPONENT_NAME,
LOG_VIEWER_WEBUI_COMPONENT_NAME,
QUEUE_COMPONENT_NAME,
REDIS_COMPONENT_NAME,
REDUCER_COMPONENT_NAME,
Expand Down Expand Up @@ -494,3 +495,16 @@ def validate_webui_config(
raise ValueError(f"{WEBUI_COMPONENT_NAME} logs directory is invalid: {ex}")

validate_port(f"{WEBUI_COMPONENT_NAME}.port", clp_config.webui.host, clp_config.webui.port)


def validate_log_viewer_config(clp_config: CLPConfig, logs_dir: pathlib.Path):
try:
validate_path_could_be_dir(logs_dir)
except ValueError as ex:
raise ValueError(f"{LOG_VIEWER_WEBUI_COMPONENT_NAME} logs directory is invalid: {ex}")

validate_port(
f"{LOG_VIEWER_WEBUI_COMPONENT_NAME}.port",
clp_config.log_viewer_webui.host,
clp_config.log_viewer_webui.port,
)
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
COMPRESSION_WORKER_COMPONENT_NAME,
CONTROLLER_TARGET_NAME,
DB_COMPONENT_NAME,
LOG_VIEWER_WEBUI_COMPONENT_NAME,
QUERY_JOBS_TABLE_NAME,
QUERY_SCHEDULER_COMPONENT_NAME,
QUERY_WORKER_COMPONENT_NAME,
Expand Down Expand Up @@ -49,6 +50,7 @@
validate_and_load_queue_credentials_file,
validate_and_load_redis_credentials_file,
validate_db_config,
validate_log_viewer_config,
validate_queue_config,
validate_redis_config,
validate_reducer_config,
Expand Down Expand Up @@ -698,10 +700,9 @@ def start_webui(instance_id: str, clp_config: CLPConfig, mounts: CLPDockerMounts
return

webui_logs_dir = clp_config.logs_directory / component_name
node_path = str(
CONTAINER_CLP_HOME / "var" / "www" / "programs" / "server" / "npm" / "node_modules"
)
settings_json_path = get_clp_home() / "var" / "www" / "settings.json"
container_webui_dir = CONTAINER_CLP_HOME / "var" / "www" / "webui"
node_path = str(container_webui_dir / "programs" / "server" / "npm" / "node_modules")
settings_json_path = get_clp_home() / "var" / "www" / "webui" / "settings.json"

validate_webui_config(clp_config, webui_logs_dir, settings_json_path)

Expand Down Expand Up @@ -758,9 +759,67 @@ def start_webui(instance_id: str, clp_config: CLPConfig, mounts: CLPDockerMounts
container_cmd.append(clp_config.execution_container)

node_cmd = [
str(CONTAINER_CLP_HOME / "bin" / "node"),
str(CONTAINER_CLP_HOME / "var" / "www" / "launcher.js"),
str(CONTAINER_CLP_HOME / "var" / "www" / "main.js"),
str(CONTAINER_CLP_HOME / "bin" / "node-14"),
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved
str(container_webui_dir / "launcher.js"),
str(container_webui_dir / "main.js"),
]
cmd = container_cmd + node_cmd
subprocess.run(cmd, stdout=subprocess.DEVNULL, check=True)

logger.info(f"Started {component_name}.")


def start_log_viewer_webui(instance_id: str, clp_config: CLPConfig, mounts: CLPDockerMounts):
component_name = LOG_VIEWER_WEBUI_COMPONENT_NAME
logger.info(f"Starting {component_name}...")

container_name = f"clp-{component_name}-{instance_id}"
if container_exists(container_name):
return

log_viewer_webui_logs_dir = clp_config.logs_directory / component_name
container_log_viewer_dir = CONTAINER_CLP_HOME / "var" / "www" / "log_viewer"
node_path = str(container_log_viewer_dir / "server" / "node_modules")

validate_log_viewer_config(clp_config, log_viewer_webui_logs_dir)

# Create directories
log_viewer_webui_logs_dir.mkdir(exist_ok=True, parents=True)

container_log_viewer_webui_logs_dir = pathlib.Path("/") / "var" / "log" / component_name

# Start container
# fmt: off
container_cmd = [
"docker", "run",
"-d",
"--network", "host",
"--name", container_name,
"--log-driver", "local",
"-e", f"NODE_PATH={node_path}",
"-e", f"CLIENT_DIR=../client/dist",
"-e", f"PORT={clp_config.log_viewer_webui.port}",
"-e", f"HOST={clp_config.log_viewer_webui.host}",
"-e", f"NODE_ENV=production",
"-u", f"{os.getuid()}:{os.getgid()}",
]
# fmt: on
necessary_mounts = [
mounts.clp_home,
mounts.ir_output_dir,
DockerMount(
DockerMountType.BIND, log_viewer_webui_logs_dir, container_log_viewer_webui_logs_dir
),
]
for mount in necessary_mounts:
if mount:
container_cmd.append("--mount")
container_cmd.append(str(mount))
container_cmd.append(clp_config.execution_container)

node_cmd = [
str(CONTAINER_CLP_HOME / "bin" / "node-22"),
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved
str(container_log_viewer_dir / "server" / "src" / "main.js"),
]
cmd = container_cmd + node_cmd
subprocess.run(cmd, stdout=subprocess.DEVNULL, check=True)
Expand Down Expand Up @@ -897,6 +956,7 @@ def main(argv):
COMPRESSION_SCHEDULER_COMPONENT_NAME,
QUERY_SCHEDULER_COMPONENT_NAME,
WEBUI_COMPONENT_NAME,
LOG_VIEWER_WEBUI_COMPONENT_NAME,
):
validate_and_load_db_credentials_file(clp_config, clp_home, True)
if target in (
Expand Down Expand Up @@ -984,6 +1044,8 @@ def main(argv):
start_reducer(instance_id, clp_config, container_clp_config, num_workers, mounts)
if target in (ALL_TARGET_NAME, WEBUI_COMPONENT_NAME):
start_webui(instance_id, clp_config, mounts)
if target in (ALL_TARGET_NAME, LOG_VIEWER_WEBUI_COMPONENT_NAME):
start_log_viewer_webui(instance_id, clp_config, mounts)

except Exception as ex:
if type(ex) == ValueError:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
COMPRESSION_WORKER_COMPONENT_NAME,
CONTROLLER_TARGET_NAME,
DB_COMPONENT_NAME,
LOG_VIEWER_WEBUI_COMPONENT_NAME,
QUERY_SCHEDULER_COMPONENT_NAME,
QUERY_WORKER_COMPONENT_NAME,
QUEUE_COMPONENT_NAME,
Expand Down Expand Up @@ -134,6 +135,9 @@ def main(argv):

already_exited_containers = []
force = parsed_args.force
if target in (ALL_TARGET_NAME, LOG_VIEWER_WEBUI_COMPONENT_NAME):
container_name = f"clp-{LOG_VIEWER_WEBUI_COMPONENT_NAME}-{instance_id}"
stop_running_container(container_name, already_exited_containers, force)
if target in (ALL_TARGET_NAME, WEBUI_COMPONENT_NAME):
container_name = f"clp-{WEBUI_COMPONENT_NAME}-{instance_id}"
stop_running_container(container_name, already_exited_containers, force)
Expand Down
25 changes: 25 additions & 0 deletions components/clp-py-utils/clp_py_utils/clp_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
COMPRESSION_WORKER_COMPONENT_NAME = "compression_worker"
QUERY_WORKER_COMPONENT_NAME = "query_worker"
WEBUI_COMPONENT_NAME = "webui"
LOG_VIEWER_WEBUI_COMPONENT_NAME = "log_viewer_webui"

# Target names
ALL_TARGET_NAME = ""
Expand Down Expand Up @@ -382,6 +383,29 @@ def validate_logging_level(cls, field):
return field


class LogViewerWebUi(BaseModel):
host: str = "localhost"
port: int = 3000
logging_level: str = "INFO"

@validator("host")
def validate_host(cls, field):
if "" == field:
raise ValueError(f"{LOG_VIEWER_WEBUI_COMPONENT_NAME}.host cannot be empty.")
return field

@validator("port")
def validate_port(cls, field):
min_valid_port = 0
max_valid_port = 2**16 - 1
if min_valid_port > field or max_valid_port < field:
raise ValueError(
f"{LOG_VIEWER_WEBUI_COMPONENT_NAME}.port is not within valid range "
f"{min_valid_port}-{max_valid_port}."
)
return field
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved


class CLPConfig(BaseModel):
execution_container: typing.Optional[str]

Expand All @@ -398,6 +422,7 @@ class CLPConfig(BaseModel):
compression_worker: CompressionWorker = CompressionWorker()
query_worker: QueryWorker = QueryWorker()
webui: WebUi = WebUi()
log_viewer_webui: LogViewerWebUi = LogViewerWebUi()
credentials_file_path: pathlib.Path = CLP_DEFAULT_CREDENTIALS_FILE_PATH

archive_output: ArchiveOutput = ArchiveOutput()
Expand Down
4 changes: 4 additions & 0 deletions components/package-template/src/etc/clp-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@
# port: 4000
# logging_level: "INFO"
#
#log_viewer_webui
# host: "localhost"
# port: 3000
#
## Where archives should be output to
#archive_output:
# directory: "var/data/archives"
Expand Down