Standardizing Logging Level Strings, Fixed `do_kafka_test.sh` script #31

dmccoystephenson · 2023-09-15T15:45:43Z

Logging Level Strings

The logging level strings have been made uppercase to align with the logging level strings used in the other submodules.

Fixed `do_kafka_test.sh` Script

The do_kafka_test.sh was created six years ago, but it was discovered last year that it was not functioning properly due to inconsistencies in container name references and issues with dependent scripts.

The script has been refactored and thoroughly tested to ensure its functionality when executed in WSL.

Additional Changes

The dockerfile.testing file has been added to the project for testing purposes. This includes a python installation.
The majority of the code has been moved into methods in do_kafka_test.sh
Color has been added to the do_kafka_test.sh, do_bsm_test.sh and do_tim_test.sh scripts.
The standalone.sh and standalone-multi.sh scripts have been refactored.
Descriptions as comments have been added to a number of scripts.
The DOCKER_HOST_IP environment variable is now required to be set for the do_kafka_test.sh script to run.
The docker-compose-kafka.yml file has been added for testing purposes.
The default logging level has been changed to 'ERROR' instead of 'TRACE'.

Testing

These changes have been verified to be working when the script is run locally in WSL.

…petitive behavior.

…dability.

…ipt.

…to the `standalone.sh` script.

…one.sh` script

…time to be ready.

…ndalone`

…ut to the kafka-topics.sh script.

…topic creation check.

… PPM logs is above 100 instead of waiting a fixed amount of time for PPM readiness.

…`standalone-multi.sh`

…`do_kafka_test.sh`

…script calls in `do_bsm_test.sh` & `do_tim_test.sh`

update logging level strings

Fixed `do_kafka_test.sh` script

Update Default Logging Level

dan-du-car · 2023-10-23T18:00:29Z

.devcontainer/devcontainer.json

@@ -19,6 +19,13 @@
 		"ms-vscode.cmake-tools"
 	],

+	"features": {
+		"docker-from-docker": {


which repository does this feature reference to?

There is a warning for this feature.

Not sure it is just my local environment. Sometimes the code take some time to run with the feature on and the terminal does not return control to user.

dan-du-car · 2023-10-23T18:10:47Z

Dockerfile.testing

+ADD ./docker-test /cvdi-stream/docker-test
+
+# Run the tool.
+RUN chmod 7777 /cvdi-stream/docker-test/ppm_no_map.sh


Is this typo "7777"?

dan-du-car · 2023-10-23T20:30:30Z

Dockerfile.testing

+ADD ./config /cvdi-stream/config
+
+# Do the build.
+RUN export LD_LIBRARY_PATH=/usr/local/lib && mkdir /cvdi-stream-build && cd /cvdi-stream-build && cmake /cvdi-stream && make


Is this /cvdi-stream-build folder or a build folder inside /cvdi-stream? like /cvdi-stream/build mkdir -p /cvdi-stream/build && cd /cvdi-stream/build && cmake /cvdi-stream

Can we reuse the ./devcontainer and replace the Dockerfile.testing with the vscode test task for the testing?

Project files are stored in /cvdi-stream and are used to build executables in /cvdi-stream-build.

The Dockerfile.testing file is used by scripts involved in the execution of the do_kafka_test.sh script, which is an integration test between the PPM & Kafka. The primary difference is that python is included so that the 'test_in.py' and 'test_out.py' scripts are usable.

I don't think this should be replaced by using the dev container, since the dev container is meant for development and the Dockerfile.testing file is meant for integration testing.

…ng opt string.

…of passing opt string." This reverts commit fb010fa.

dan-du-car · 2023-10-24T17:08:37Z

src/ppm.cpp

@@ -245,19 +245,19 @@ void PPM::print_configuration() const
 bool PPM::configure() {

    if ( optIsSet('v') ) {
-        if ( "trace" == optString('v') ) {
+        if ( "TRACE" == optString('v') ) {


Can we make this log level string case insensitive? I see the configuration doc mentioned

-v | --log-level : The info log level [trace,debug,info,warning,error,critical,off]

They are all lower case. The code only compare the upper case.

dan-du-car · 2023-10-24T17:12:22Z

start_kafka.sh

-docker-compose rm -f -v
-docker-compose up --build -d
-docker ps
+./stop_kafka.sh


It's possible that 'chmod 777' needs to be executed on stop_kafka.sh prior to using the start_kafka.sh script here.

yeah, adding 777 works for me.

dan-du-car · 2023-10-24T19:02:18Z

docker-test/do_bsm_test.sh

-    /cvdi-stream-build/kafka-test/kafka_tool -C -b $broker -p 0 -t topic.FilteredOdeBsmJson -e -o $offset 2> con.err | /cvdi-stream/docker-test/test_out.py > tmp.out
+    attempts=$((attempts+1))
+
+    timeout 5 /cvdi-stream-build/kafka-test/kafka_tool -C -b $broker -p 0 -t topic.FilteredOdeBsmJson -e -o $offset 2> con.err | /cvdi-stream/docker-test/test_out.py > tmp.out


The pipe break and it is not sending data to Kafka.

The do_bsm_test.sh script isn't intended to be run directly. I believe it is used by the standalone.sh and standalone_multi.sh scripts, which are in turn used by the do_kafka_test.sh script. Do you experience this behavior when running the do_kafka_test.sh script?

I am not able to finish running the do_kafka_test.sh as some data are missing below (or I do not have those data in my ubuntu):

/ppm_data/road_file.csv /tmp/docker-test/data

dan-du-car · 2023-10-24T19:17:38Z

do_kafka_test.sh


+CURRENT_DIR_NAME=${PWD##*/}


Test Scenario 1:
I started Zookeeper and Kafka container first, then run this do_kafka_test.sh script. This script is going to start kafka again in the setup() func, and waitForKafkaToCreateTopics() complain that Kafka is not up.

Test Scenario 2:
Stop Kafka and zookeeper. Run the do_kafka_test.sh script.

IT will start the containers and then stop them at step 2. Should we remove the stop_kafka.sh from the start_kafka.sh script?

If Zookeeper and Kafka are already running, the do_kafka_test.sh script should restart them prior to building the PPM image and running the tests.

If Zookeeper and Kafka are not running, the do_kafka_test.sh script should start them.

Another comment of yours mentions that the kafka container name uses an underscore, not a hyphen. It's possible that this is why the container is not being found in Step 2. Interestingly, when I run the script the container name uses a hyphen, not an underscore. I wonder if this could be due to differences in docker versions. I am using Docker Desktop 4.17.0, what are you using?

I am using ubuntu 18.04 and docker version 20.10.6

dan-du-car · 2023-10-24T19:21:40Z

do_kafka_test.sh

+    # print setup info
+    echo "=== Setup Info ==="
+    echo "DOCKER_HOST_IP: $DOCKER_HOST_IP"
+    echo "KAFKA_CONTAINER_NAME: $KAFKA_CONTAINER_NAME"


The kafka container name is separated by underscore not hyphen.

On my machine, the kafka container name is separated by a hyphen. This may have to do with differences in Docker Desktop versions. I am using v4.17.0, what are you using?

We may want to consider modifying the script to check for container names separated by an underscore and a hyphen, just to cover all of our bases.

dan-du-car · 2023-10-24T19:39:32Z

test-scripts/standalone.sh

+        fi
+    done
+    echo "Starting PPM in new container '$PPM_CONTAINER_NAME'"
+    docker run --name $PPM_CONTAINER_NAME --env DOCKER_HOST_IP=$dockerHostIp --env PPM_LOG_TO_CONSOLE=true --env PPM_LOG_TO_FILE=true -v /tmp/docker-test/data:/ppm_data -d -p '8080:8080' $PPM_IMAGE_NAME:$PPM_IMAGE_TAG /cvdi-stream/docker-test/ppm_standalone.sh


Can we use relative path for testing? The container is created but not running.

Since the standalone.sh script in intended to run inside of a freshly-spun-up docker container, I believe using an absolute path here is the way to go.

The reason that you are experiencing stalling is because the script is expecting the container to have a number of logs before it is deemed 'ready'. Recently, we switched the default logging level to ERROR instead of TRACE, so the container is not spitting out any logs. This should be resolved by CDOT-CV#36. It should be noted that CDOT#36 is not merged into develop yet, so I suggest trying this again once it has been merged.

It makes sense as /cvdi-stream is inside container. Would like to clarify that I am not referring to /cvdi-stream/. I am running the standalone.sh inside my ubuntu, and I do not have this /tmp/docker-test/data directory. I am wandering if we could reuse the data directory in this repository.

dan-du-car · 2023-10-24T19:40:48Z

test-scripts/standalone.sh

 if [ $4 = "BSM" ]; then
-    docker exec ppm_kafka /cvdi-stream/docker-test/do_bsm_test.sh $OFFSET
-    # Produce the test data.
+    docker exec $PPM_CONTAINER_NAME /cvdi-stream/docker-test/do_bsm_test.sh $OFFSET


The same here. It might be helpful to use relative path rather than absolute path.

Since the command is intended to run inside of a docker container, an absolute path should be a fine method for referencing the script. I don't think this is a problem.

dmccoystephenson added 30 commits April 13, 2022 15:23

Made some changes to resolve some issues with do_kafka_test.sh script.

970f1c2

Merge branch 'develop' into fixing-kafka-test-script

03440ab

Refactored scripts to improve clarity of execution and to minimize re…

11600d9

…petitive behavior.

Organized code in do_kafka_test.sh script into methods to improve rea…

57aebcc

…dability.

Made the DOCKER_HOST_IP env var required for the do_kafka_test.sh scr…

2496024

…ipt.

Modified topic creation check in do_kafka_test.sh script.

7d6d8c8

Moved the responsibility of starting/stopping the PPM container back …

19af792

…to the `standalone.sh` script.

Modified & added color to echo messages.

c6bb676

Gave the PPM container an arbitrary 10 seconds to spin up in `standal…

58ae4c8

…one.sh` script

Refactored standalone_multi.sh script and gave PPM containers more …

cb3b5a9

…time to be ready.

Added descriptions as comments to a number of scripts.

2685f49

Removed duplicate python installation in Dockerfile.standalone

adfb334

Created separate dockerfile for testing.

b8b1124

Removed python installation from Dockerfile-nsv and `Dockerfile.sta…

4dbfdb0

…ndalone`

Added documentation for the do_kafka_test.sh script to the README.

96976ff

Merge branch 'develop' into fixing-kafka-test-script

9dfa167

Modified 'Kafka Test Script' section of README.

42cda70

Modified Build & Exec Script section of the README.

a22253b

Added note on using the LF EOF sequence for scripts to the README.

f1a4e4c

Updated base image in dockerfiles to ubuntu:20.04

1ab0ac7

Merge branch 'develop' into fixing-kafka-test-script

8c21aa4

Copied 'config' directory over to the image during build.

7b362ac

Switched back to using ubuntu:18.04 for the base image.

30ca2af

Switched over to using $DOCKER_HOST_IP instead of 172.17.0.1 as inp…

0300a34

…ut to the kafka-topics.sh script.

Added a note regarding WSL hanging to the README.

84e5153

Switched back to using 172.17.0.1 instead of $DOCKER_HOST_IP for the …

af674cf

…topic creation check.

Modified standalone test scripts to wait until the number of lines of…

76de636

… PPM logs is above 100 instead of waiting a fixed amount of time for PPM readiness.

Modified note about WSL hanging in the README.

9f4b90b

Modified PPM container name in do_kafka_test.sh, standalone.sh & …

144a91f

…`standalone-multi.sh`

Used name of current directory when defining kafka container name in …

69af1ac

…`do_kafka_test.sh`

mwodahl and others added 5 commits July 18, 2023 12:30

update logging level strings

b3c1240

Printed setup info in do_kafka_test.sh and added timeout to python …

bb0f062

…script calls in `do_bsm_test.sh` & `do_tim_test.sh`

Merge pull request #31 from CDOT-CV/logging-strings

af17a5e

update logging level strings

Update default logging value for PPM

59e3bde

Merge pull request #28 from CDOT-CV/fixing-kafka-test-script

bec5045

Fixed `do_kafka_test.sh` script

dmccoystephenson changed the title ~~Standardizing Logging Level Strings~~ Standardizing Logging Level Strings, Fixed do_kafka_test.sh script Sep 27, 2023

Merge pull request #33 from CDOT-CV/default-logging-level

6705ceb

Update Default Logging Level

dan-du-car reviewed Oct 23, 2023

View reviewed changes

dmccoystephenson added 2 commits October 24, 2023 10:52

Referenced PPM_LOG_LEVEL in PpmLogger class directly instead of passi…

fb010fa

…ng opt string.

Revert "Referenced PPM_LOG_LEVEL in PpmLogger class directly instead …

d1ae139

…of passing opt string." This reverts commit fb010fa.

dan-du-car reviewed Oct 24, 2023

View reviewed changes

This was referenced Oct 24, 2023

Direct PPM_LOG_LEVEL Access and Stalling Prevention CDOT-CV/jpo-cvdp#36

Merged

Modifying dockerfiles & .devcontainer CDOT-CV/jpo-cvdp#34

Merged

dan-du-car reviewed Oct 24, 2023

View reviewed changes

dan-du-car approved these changes Oct 30, 2023

View reviewed changes

dan-du-car merged commit 74e24f7 into usdot-jpo-ode:develop Oct 30, 2023
1 check passed

dmccoystephenson mentioned this pull request Oct 30, 2023

Direct PPM_LOG_LEVEL Access and Stalling Prevention #33

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Standardizing Logging Level Strings, Fixed `do_kafka_test.sh` script #31

Standardizing Logging Level Strings, Fixed `do_kafka_test.sh` script #31

dmccoystephenson commented Sep 15, 2023 •

edited

Loading

dan-du-car Oct 23, 2023 •

edited

Loading

dan-du-car Oct 24, 2023 •

edited

Loading

dan-du-car Oct 24, 2023

dan-du-car Oct 23, 2023

dan-du-car Oct 23, 2023

dan-du-car Oct 23, 2023

dmccoystephenson Oct 24, 2023

dmccoystephenson Oct 24, 2023 •

edited

Loading

dan-du-car Oct 24, 2023 •

edited

Loading

dan-du-car Oct 24, 2023

dmccoystephenson Oct 24, 2023

dan-du-car Oct 24, 2023

dan-du-car Oct 24, 2023

dmccoystephenson Oct 25, 2023

dan-du-car Oct 25, 2023

dan-du-car Oct 24, 2023

dmccoystephenson Oct 25, 2023

dan-du-car Oct 25, 2023

dan-du-car Oct 24, 2023

dmccoystephenson Oct 25, 2023

dan-du-car Oct 24, 2023

dmccoystephenson Oct 25, 2023

dan-du-car Oct 25, 2023

dan-du-car Oct 24, 2023

dmccoystephenson Oct 25, 2023


		CURRENT_DIR_NAME=${PWD##*/}

Standardizing Logging Level Strings, Fixed do_kafka_test.sh script #31

Standardizing Logging Level Strings, Fixed do_kafka_test.sh script #31

Conversation

dmccoystephenson commented Sep 15, 2023 • edited Loading

Logging Level Strings

Fixed do_kafka_test.sh Script

Additional Changes

Testing

dan-du-car Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

dan-du-car Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmccoystephenson Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

dan-du-car Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Standardizing Logging Level Strings, Fixed `do_kafka_test.sh` script #31

Standardizing Logging Level Strings, Fixed `do_kafka_test.sh` script #31

dmccoystephenson commented Sep 15, 2023 •

edited

Loading

Fixed `do_kafka_test.sh` Script

dan-du-car Oct 23, 2023 •

edited

Loading

dan-du-car Oct 24, 2023 •

edited

Loading

dmccoystephenson Oct 24, 2023 •

edited

Loading

dan-du-car Oct 24, 2023 •

edited

Loading