Setup docker template access to EFS #302

laspsandoval · 2024-01-03T19:26:12Z

Change Summary

Overview

Sets up docker template that can be used to push images to the ECR, provides instructions of how to do that, and provides access to EFS containing SPICE kernels.

New Files

Dockerfile
- Dockerfile that installs imap_processing and its dependencies
docker.rst
- Provides description of how to build and run docker image. Also contains information of how to use the image in the ECR

Deleted Files

None

Updated Files

run_processing.py
- I added a really brief example of how spice kernels from the ECR could be accessed.
index.rst
- Added docker.rst to toctree.
poetry.lock
- Updated

Testing

I made some minor changes to sds-data-manager, which I will describe in another PR for that repo. I then populated the EFS using the lambda Tenzin created (worked beautifully by the way). And then I executed a Step Function manually and looked at the Batch Job log. Picture is attached.

greglucas

Shouldn't this be in sds-data-manager? You have AWS information here, and the processing repository should be agnostic to any AWS work and able to be run locally.

I'm not following why we need all these steps... pip install imap_processing should get us all the scripts we need to do any processing steps. So it should be a 2-3 line Dockerfile I'd hope. (You can also remove the install git if you change to install from a zip archive for now)

The script should be installed and accessed as imap_processing --flags.
If tools/ are needed during processing, then those should probably be moved up into the main package to get their dependencies automatically?

tech3371 · 2024-01-04T16:24:22Z

Dockerfile

+
+# Copy over only the necessary scripts
+COPY imap_processing/run_processing.py $IMAP_PROCESS_DIRECTORY/run_processing.py
+COPY tools $IMAP_PROCESS_DIRECTORY/tools


Do we need to copy tools folder? I think it's only used locally to create XTCE.

I removed the example I had of how to use the kernels, which is what I was using the tools folder for. But I removed the example.

tech3371 · 2024-01-04T16:25:09Z

Dockerfile

+RUN pip install git+https://github.com/IMAP-Science-Operations-Center/imap_processing.git@dev
+
+# Copy over only the necessary scripts
+COPY imap_processing/run_processing.py $IMAP_PROCESS_DIRECTORY/run_processing.py


I think it should come in the pip package. If it didn't, we could find out why.

tech3371 · 2024-01-04T16:30:23Z

Dockerfile

+RUN mkdir -p /mnt/spice
+
+# Define the entrypoint of the container
+ENTRYPOINT ["python", "/opt/imap/run_processing.py"]


I haven't tested it but I wonder if you could change this to the pip path. Then you won't need to copy in above line.

Suggested change

ENTRYPOINT ["python", "/opt/imap/run_processing.py"]

ENTRYPOINT ["python", "<pip path>/imap_processing/run_processing.py"]

I believe you can get by doing pip show imap_processing

tech3371 · 2024-01-04T16:33:11Z

docs/source/development-guide/docker.rst

+To build the image run the following command from the directory containing the Dockerfile. You might add -t option to tag your image
+and --rm to remove intermediate containers after the build is done.
+
+    `docker build -t <image name> --rm .`


Suggested change

`docker build -t <image name> --rm .`

`docker build -t <image name>:<tag name> --rm .`

tech3371 · 2024-01-04T16:34:21Z

docs/source/development-guide/docker.rst

+
+Now we can run our image.
+
+    `docker run --rm -it --volume="$(pwd)/imap_processing/efs:/mnt/spice" <image name> --instrument <instrument> --level <level>`


Suggested change

`docker run --rm -it --volume="$(pwd)/imap_processing/efs:/mnt/spice" <image name> --instrument <instrument> --level <level>`

`docker run --rm -it --volume="$(pwd)/imap_processing/efs:/mnt/spice" <image name>:<tag name> --instrument <instrument> --level <level>`

tech3371 · 2024-01-04T16:36:26Z

docs/source/development-guide/docker.rst

+Build the Docker image.
+
+    `docker build -t <image name> .`
+
+Tag the image and push to the ECR.
+
+    `docker tag <tag> <ECR URI>`
+
+    `docker push <ECR URI>`


Is this the place where we would set image tag with version?

We can assign the tag name during the build :)

tech3371

looks good to me. Thank you!

tech3371 · 2024-01-04T20:03:36Z

imap_processing/cli.py

@@ -36,11 +37,10 @@ def _parse_args():
        f"The data level to process. Acceptable values are: {processing_levels}"
    )

-    parser = argparse.ArgumentParser(description=description)
+    parser = argparse.ArgumentParser(prog="imap_cli", description=description)


I was wondering what this was but looks like it was from poetry. cool

greglucas

OK, since this is mostly documentation at this point I'm fine with keeping it in this repository since it is more public-facing. But, I think the Dockerfile should be moved out of our base repository level.

greglucas · 2024-01-04T21:43:10Z

imap_processing/cli.py

@@ -7,17 +7,18 @@

 Use
 ---
-    python run_processing.py <instrument> <data_level>
+    python cli.py <instrument> <data_level>


imap_cli --instrument <instrument> --level <data_level>

greglucas · 2024-01-04T21:44:16Z

imap_processing/cli.py

@@ -162,11 +162,12 @@ def process(self):
        print(f"Processing IMAP-Ultra {self.level}")


-if __name__ == "__main__":


You still need the if __name__ == "__main__" call main() block for running it as python cli.py` So it is best to leave that in there for either option.

greglucas · 2024-01-04T21:45:08Z

Dockerfile

I'd prefer this doesn't live at the root level. Maybe move it into an examples/ folder?

Also perhaps call it some_unique_name.Dockerfile or Dockerfile.some_unique_name so it isn't just a base Dockerfile?

greglucas

Thanks for the clear explanations and descriptions, I think this is nice with the explicit example.

I would still like to see the cli_args removed if that isn't actually needed/used.

greglucas · 2024-01-05T02:09:17Z

imap_processing/cli.py


 from imap_processing import instruments, processing_levels


-def _parse_args():
+def _parse_args(cli_args: list):


Why do we need the cli_args here and in main()? I was pretty sure in my testing that there was not extra args passed into the script's main function call...

https://docs.python.org/3/library/argparse.html#parsing-arguments

In a script, parse_args() will typically be called with no arguments, and the ArgumentParser will automatically determine the command-line arguments from sys.argv.

greglucas · 2024-01-05T02:10:42Z

docs/source/development-guide/docker.rst

+To build the image run the following command from the directory containing the Dockerfile. You might add -t option to tag your image
+and --rm to remove intermediate containers after the build is done.
+
+    `docker build -t <image name>:<tag name> --rm .`


Do you want to give an example of how to build the image you added? You'll need the -f Dockerfile.name now because it isn't the default Dockerfile (sorry for the misdirection there)!

Suggested change

`docker build -t <image name>:<tag name> --rm .`

`docker build -f examples/Dockerfile.efs -t <image name>:<tag name> --rm .`

bourque · 2024-01-31T16:53:31Z

@all-contributors please add @laspsandoval for infrastructure and ideas

allcontributors · 2024-01-31T16:53:40Z

@bourque

I've put up a pull request to add @laspsandoval! 🎉

* Setup docker template access to EFS

laspsandoval linked an issue Jan 3, 2024 that may be closed by this pull request

Setup docker template access to EFS #242

Closed

laspsandoval requested review from bourque, greglucas, tech3371, maxinelasp, sdhoyt and bryan-harter January 3, 2024 20:00

greglucas reviewed Jan 3, 2024

View reviewed changes

laspsandoval mentioned this pull request Jan 3, 2024

update to batch env IMAP-Science-Operations-Center/sds-data-manager#212

Merged

tech3371 reviewed Jan 4, 2024

View reviewed changes

tech3371 approved these changes Jan 4, 2024

View reviewed changes

greglucas reviewed Jan 4, 2024

View reviewed changes

greglucas approved these changes Jan 5, 2024

View reviewed changes

laspsandoval added 17 commits January 8, 2024 13:11

adding mounting to script

592f85f

first pass at EFS mount

bf6447b

add Dockerfile

31cb3f3

dockerfile

61e8fa2

adding Dockerfile

0633c36

changes to run_processing.py

9ad0413

additional Dockerfile edits

ecfcfe5

added short example for spice

a1da2f3

update to env

dc3b96c

added to toctree

8ac7ba3

test

b178dec

pyproject.toml updates

ac70464

testing

d0e65fd

test

d162002

test

316a14d

test

e3a294f

test

f0fbcc9

laspsandoval added 5 commits January 8, 2024 13:11

updates to Dockerfile

7094d9a

change to rst

083e0d9

change Dockerfile location and minor cli changes

4706d45

PR comment response

aa21524

PR comment response

191f966

laspsandoval force-pushed the dev branch from dcc6893 to 191f966 Compare January 8, 2024 20:12

laspsandoval merged commit 62b3703 into IMAP-Science-Operations-Center:dev Jan 8, 2024
14 checks passed

allcontributors bot mentioned this pull request Jan 31, 2024

docs: add laspsandoval as a contributor for infra, and ideas #322

Merged

laspsandoval self-assigned this Feb 7, 2024

laspsandoval added a commit to laspsandoval/imap_processing that referenced this pull request Apr 2, 2024

Setup docker template access to EFS (IMAP-Science-Operations-Center#302)

f629bd8

* Setup docker template access to EFS

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup docker template access to EFS #302

Setup docker template access to EFS #302

laspsandoval commented Jan 3, 2024 •

edited

Loading

greglucas left a comment

tech3371 Jan 4, 2024

laspsandoval Jan 4, 2024

tech3371 Jan 4, 2024

tech3371 Jan 4, 2024

tech3371 Jan 4, 2024

tech3371 Jan 4, 2024

tech3371 Jan 4, 2024

laspsandoval Jan 4, 2024

tech3371 left a comment

tech3371 Jan 4, 2024

greglucas left a comment

greglucas Jan 4, 2024

greglucas Jan 4, 2024

greglucas Jan 4, 2024

greglucas left a comment

greglucas Jan 5, 2024

greglucas Jan 5, 2024

bourque commented Jan 31, 2024

allcontributors bot commented Jan 31, 2024

	ENTRYPOINT ["python", "/opt/imap/run_processing.py"]
	ENTRYPOINT ["python", "<pip path>/imap_processing/run_processing.py"]

	`docker build -t <image name> --rm .`
	`docker build -t <image name>:<tag name> --rm .`


		Now we can run our image.

		`docker run --rm -it --volume="$(pwd)/imap_processing/efs:/mnt/spice" <image name> --instrument <instrument> --level <level>`

		@@ -162,11 +162,12 @@ def process(self):
		print(f"Processing IMAP-Ultra {self.level}")


		if __name__ == "__main__":

	`docker build -t <image name>:<tag name> --rm .`
	`docker build -f examples/Dockerfile.efs -t <image name>:<tag name> --rm .`

Setup docker template access to EFS #302

Setup docker template access to EFS #302

Conversation

laspsandoval commented Jan 3, 2024 • edited Loading

Change Summary

Overview

New Files

Deleted Files

Updated Files

Testing

greglucas left a 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

tech3371 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greglucas left a 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

greglucas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bourque commented Jan 31, 2024

allcontributors bot commented Jan 31, 2024

laspsandoval commented Jan 3, 2024 •

edited

Loading