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.

Sign up for GitHub

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

MTV-1972: Fixing start script for macos #1427

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

MTV-1972: Fixing start script for macos #1427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2a2b25
Fixing start script to work on mac
metalice Feb 10, 2025
9e4603b
update script
metalice Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 34 additions & 20 deletions ci/start-console.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,39 @@ set -euo pipefail
script_dir=$(dirname "$0")
source ${script_dir}/configure/openshift.sh

CONSOLE_CONTAINER_NAME=okd-console
FORKLIFT_NAMESPACE=konveyor-forklift
CONSOLE_CONTAINER_NAME="okd-console"
FORKLIFT_NAMESPACE="konveyor-forklift"

BASE_HOST_URL="https://localhost"

PLUGIN_NAME="forklift-console-plugin"
PLUGIN_URL=${PLUGIN_URL:-"http://localhost:9001"}
CONTAINER_NETWORK_TYPE=${CONTAINER_NETWORK_TYPE:-"host"}
CONSOLE_IMAGE=${CONSOLE_IMAGE:-"quay.io/openshift/origin-console:latest"}
CONSOLE_PORT=${CONSOLE_PORT:-9000}
PLUGIN_URL="http://localhost:9001"

CONTAINER_NETWORK="--network=host"

CONSOLE_IMAGE="quay.io/openshift/origin-console:latest"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
CONSOLE_IMAGE="quay.io/openshift/origin-console:latest"
CONSOLE_IMAGE=${CONSOLE_IMAGE:-"quay.io/openshift/origin-console:latest"}

Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to support setting those vars externally to be backward with previous implementation - see README.

CONSOLE_PORT="9000"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
CONSOLE_PORT="9000"
CONSOLE_PORT=${CONSOLE_PORT:-9000}

Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

CONSOLE_PORT_PUBLISH="--publish=$CONSOLE_PORT:$CONSOLE_PORT"
CONSOLE_CONTAINER_NAME_RUN="--name=$CONSOLE_CONTAINER_NAME"

# On macOS
if [[ $(uname) = "Darwin" ]]; then
BASE_HOST_URL="http://host.containers.internal"
CONTAINER_NETWORK=""
PLUGIN_URL="$BASE_HOST_URL:9001"
fi

# Default to localhost
INVENTORY_SERVER_HOST="$BASE_HOST_URL:30444"
SERVICES_API_SERVER_HOST="$BASE_HOST_URL:30446"
Comment on lines +30 to +31
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
INVENTORY_SERVER_HOST="$BASE_HOST_URL:30444"
SERVICES_API_SERVER_HOST="$BASE_HOST_URL:30446"
INVENTORY_SERVER_HOST=${INVENTORY_SERVER_HOST:-"$BASE_HOST_URL:30444"}
SERVICES_API_SERVER_HOST=${SERVICES_API_SERVER_HOST:-"$BASE_HOST_URL:30446"}

Copy link
Collaborator

Choose a reason for hiding this comment

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

same here


# Look for forklift routes
if oc_available_loggedin; then
routes=$(oc get routes -A -o template --template='{{range .items}}{{.spec.host}}{{"\n"}}{{end}}' 2>/dev/null || true)
INVENTORY_SERVER_HOST=${INVENTORY_SERVER_HOST:-$(echo "$routes" | grep forklift-inventory || true)}
SERVICES_API_SERVER_HOST=${SERVICES_API_SERVER_HOST:-$(echo "$routes" | grep forklift-services || true)}
INVENTORY_SERVER_HOST="https://$(echo "$routes" | grep forklift-inventory)"
SERVICES_API_SERVER_HOST="https://$(echo "$routes" | grep forklift-services)"
Comment on lines +36 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, worth to support defining those variables externally...

fi

# Default to localhost if no route found
INVENTORY_SERVER_HOST=${INVENTORY_SERVER_HOST:-"https://localhost:30444"}
SERVICES_API_SERVER_HOST=${SERVICES_API_SERVER_HOST:-"https://localhost:30446"}

if [[ ${CONSOLE_IMAGE} =~ ^localhost/ ]]; then
PULL_POLICY="never"
else
Expand All @@ -32,8 +45,8 @@ fi

# Test if console is already running
if podman container exists ${CONSOLE_CONTAINER_NAME}; then
echo "container named ${CONSOLE_CONTAINER_NAME} is running, exit."
exit 1
echo "container named ${CONSOLE_CONTAINER_NAME} is running, exit."
exit 1
fi

# Base setup for the bridge
Expand All @@ -48,8 +61,9 @@ fi
# When the container network is default, host.containers.internal == container host
#
# NOTE: When running KinD we should use host network type because KinD only listen on localhost.
BRIDGE_PLUGINS="${PLUGIN_NAME}=${PLUGIN_URL}"
BRIDGE_PLUGIN_PROXY=$(cat << END | jq -c .
BRIDGE_PLUGINS="$PLUGIN_NAME=$PLUGIN_URL"
BRIDGE_PLUGIN_PROXY=$(
cat <<END | jq -c .
{"services":[
{
"consoleAPIPath":"/api/proxy/plugin/${PLUGIN_NAME}/forklift-inventory/",
Expand Down Expand Up @@ -94,10 +108,10 @@ $(echo ${BRIDGE_PLUGIN_PROXY} | jq .)
podman run \
--pull=${PULL_POLICY} \
--rm \
${mount_tmp_dir_flag} \
--network=${CONTAINER_NETWORK_TYPE} \
--publish=${CONSOLE_PORT}:${CONSOLE_PORT} \
--name=${CONSOLE_CONTAINER_NAME} \
--env "BRIDGE_*" \
--arch=amd64 \
${mount_tmp_dir_flag} \
${CONTAINER_NETWORK} \
${CONSOLE_PORT_PUBLISH} \
${CONSOLE_CONTAINER_NAME_RUN} \
${CONSOLE_IMAGE}
2 changes: 1 addition & 1 deletion packages/forklift-console-plugin/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"clean:all": "yarn clean ./node_modules ./.rollup.cache",
"i18n": "i18next \"./src/**/*.{js,jsx,ts,tsx}\" [-oc] -c ./i18next-parser.config.js",
"build": "NODE_ENV=production webpack",
"start": "NODE_ENV=development webpack serve",
"start": "NODE_ENV=development webpack serve --progress",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we omit the --progress flag before merging?
It makes the webpack output longer which is .annoying...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, of course, I can omit it. I've added it :) It's just to know the percentage progress when u sit and wait to 100% to start working. with all other projects we do have it with progress

Copy link
Collaborator

Choose a reason for hiding this comment

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

And isn't it extending the build running time? It seems like it is for me :)
https://webpack.js.org/guides/build-performance/#progress-plugin

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I dont think so, and if so, it's so minor compared to the status awarnance

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we omit the --progress flag before merging?

?

"buildlib": "rollup -c --bundleConfigAsCjs",
"lint": "eslint . && stylelint \"src/**/*.css\" --allow-empty-input",
"lint:fix": "eslint . --fix && stylelint \"src/**/*.css\" --allow-empty-input --fix",
Expand Down
4 changes: 2 additions & 2 deletions packages/forklift-console-plugin/webpack.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ const config: Configuration = {
},
devServer: {
static: ['./dist'],
host: 'localhost',
hot: false,
allowedHosts: 'all',
hot: true,
port: 9001,
headers: {
'Access-Control-Allow-Origin': '*',
Expand Down
Loading