New doc: create custom application container image by MCamp859 · Pull Request #775 · clearlinux/clear-linux-documentation

MCamp859 · 2019-09-10T17:53:19Z

Signed-off-by: MCamp859 maryx.camp@intel.com

puneetse · 2019-09-10T18:30:18Z

+Overview
+********
+
+The official |CL-ATTR| container base images are published on Docker\* Hub and


Suggest making sentence 2 & 3 come before 1 as they are the primary topic of the doc.

I agree, except I recommend combining sentence 1 and 2. Something like:
"""
The official |CL-ATTR| container-based images are published as Docker images on Docker Hub [hyperlink]
"""

They are 2 different things so separate thoughts. The "official" CL docker image (i.e. https://hub.docker.com/_/clearlinux) is different from these CL-based application container images (i.e. https://hub.docker.com/u/clearlinux).

Agreed. They're separate. What I struggle with as a reader in this paragraph is: Why do I need to know this information (in original)? If you want me, as a reader, to explore these links because they're relevant to the guide, then tell me why. Both links can be included. For example, you could state: "In this guide, we use the "official" CL Docker image for the first examples. Then we use an already published image on Docker Hub [url] for the last example.

I'd like to see the Overview tied to the examples in the guide.

puneetse · 2019-09-10T18:34:49Z

+published as `Docker images on Docker Hub`_. The source for these Docker images
+is available on `GitHub`_\*.
+
+In this guide, the phrase *Clear Linux\* OS container base images* is used to


In this guide, the phrase Clear Linux* OS container base images is used to

This can be moved to the bottom of this section and make reference to custom-clear-container.rst for creating a generic CL container image.

👍 Agreed. "In this guide [...]" should follow the bullet list.

puneetse · 2019-09-10T18:43:37Z

+to edit |CL| container base images for the following situations:
+
+
+*	Customize an application image Dockerfile by adding a bundle


This whole section is redundant with the TOC and manually maintained going forward. It is better addressed by changing the TOC tree depth to 2 and ensuring good section titles.

puneetse · 2019-09-10T19:30:08Z

@@ -0,0 +1,402 @@
+.. _custom-app-container:
+
+Create customized application container images


Suggested change

Create customized application container images

Modify existing application container images

We should make a clear distinction between this and custom-clear-container.rst to avoid confusion.
custom-clear-container.rst should be a "Create from scratch" while this one is a modify/customize existing

puneetse · 2019-09-10T20:11:08Z

+Besides generating a new Docker image by editing the Dockerfile, you can
+modify a published |CL| container at runtime. This approach can
+help to accelerate the feature development process. This section describes
+steps to add Tensorflow\* into the clearlinux/python container at runtime.


Suggested change

steps to add Tensorflow\* into the clearlinux/python container at runtime.

steps to add Tensorflow\* into a clearlinux/python container at runtime, as an example.

puneetse · 2019-09-10T20:11:53Z

+
+   .. code-block:: bash
+
+	$ docker run -it --rm clearlinux/python


Suggested change

$ docker run -it --rm clearlinux/python

docker run -it --rm clearlinux/python

puneetse · 2019-09-10T20:12:05Z

+
+   .. code-block:: bash
+
+	$ docker ps


Suggested change

$ docker ps

docker ps

puneetse · 2019-09-10T20:14:21Z

+#. After the machine-learning-tensorflow bundle is installed in the container,
+   in the first console, import Tensorflow, which will be successful now. You
+   could also save the updated container using the command :command:`docker commit <Container_ID>`
+   to do further experiments on it later.


Suggested change

to do further experiments on it later.

puneetse · 2019-09-10T20:15:14Z

+		>>> tf.__version__
+		'1.13.1'
+
+#. In a third console, save the container as clearlinux/python:tensorflow_added.


Suggested change

#. In a third console, save the container as clearlinux/python:tensorflow_added.

#. In a third console, save the container with a new tag.

puneetse

Thanks for doing the conversion on this @MCamp859. Please feel free to reach out to me if any of the suggestions need clarification.

mvincerx

Afternoon @MCamp859. This guide needs some re-organization first. Then decisive edits to simplify.
The final section, Customize an application image at runtime is a good example of how numbered steps simplify and emphasize user action. Please use that as a model. In general, feel free to remove any other text that doesn't serve the objective of each example--that is, long descriptions.

mvincerx · 2019-09-11T17:52:24Z

@@ -0,0 +1,402 @@
+.. _custom-app-container:
+
+Create customized application container images


mvincerx · 2019-09-11T18:54:53Z

+Overview
+********
+
+The official |CL-ATTR| container base images are published on Docker\* Hub and


I agree, except I recommend combining sentence 1 and 2. Something like:
"""
The official |CL-ATTR| container-based images are published as Docker images on Docker Hub [hyperlink]
"""

mvincerx · 2019-09-11T18:55:16Z

+published as `Docker images on Docker Hub`_. The source for these Docker images
+is available on `GitHub`_\*.
+
+In this guide, the phrase *Clear Linux\* OS container base images* is used to


👍 Agreed. "In this guide [...]" should follow the bullet list.

mvincerx · 2019-09-11T18:56:04Z

+to edit |CL| container base images for the following situations:
+
+
+*	Customize an application image Dockerfile by adding a bundle


mvincerx · 2019-09-11T18:59:23Z

+Dockerfile:
+
+
+.. contents::


Please remove this second TOC. It's unnecessary. Top-level TOC, revised as :depth: 2, is sufficient.
Also, remove the header Examples. See my comments below for more on this.

mvincerx · 2019-09-11T19:38:17Z

+
+
+Customize an application image at runtime
+=========================================


Revise header as Example 3 ....

mvincerx · 2019-09-11T19:42:56Z

+
+Besides generating a new Docker image by editing the Dockerfile, you can
+modify a published |CL| container at runtime. This approach can
+help to accelerate the feature development process. This section describes


Revise para.
This section describes how to modify a published |CL| container at runtime. In this example, we add Tensorflow* to the :command:clearlinux/python container. This approach can help accelerate the feature development process.

mvincerx · 2019-09-11T19:44:28Z

+#. After the machine-learning-tensorflow bundle is installed in the container,
+   in the first console, import Tensorflow, which will be successful now. You
+   could also save the updated container using the command :command:`docker commit <Container_ID>`
+   to do further experiments on it later.


mvincerx · 2019-09-11T19:44:42Z

+		>>> tf.__version__
+		'1.13.1'
+
+#. In a third console, save the container as clearlinux/python:tensorflow_added.


MCamp859 · 2019-09-19T19:02:09Z

@puneetse @mvincerx Please review the latest commit.
All your feedback is done and the Overview could use another read-through...

mvincerx · 2019-09-20T17:48:34Z

I will give final feedback on this before 12 noon PST. I want to merge this by EOB today PST.
@puneetse Please make any comments before then.

mvincerx · 2019-09-20T20:08:40Z

Hi @MCamp859. After talking with @puneetse, I decided to push directly to this branch.
Please hold all commits until further knowtice.

mvincerx · 2019-09-20T21:37:45Z

@puneetse. Please search the .rst file for my embedded TODO tags. Please give more context to the other examples, similar to what we discussed in Example 1.

mvincerx · 2019-09-21T01:03:06Z

@puneetse I'll check in on this Sat. am.

puneetse · 2019-09-21T02:06:19Z

+#. Run :command:`git diff`.

-	.. code-block:: bash
+#. Next, enter this command to show the edits made after


There should be no command to enter here. It should be run`git diff`` and examine the output. It should look like this below...

mvincerx · 2019-09-23T17:53:31Z

@MCamp859, I'll take over this PR. Puneet and I can collaborate here on out. Thanks

mvincerx · 2019-09-23T23:11:31Z

@puneetse, see latest commit after CI checks are completed.

puneetse · 2019-09-24T19:00:13Z

@mvincerx thanks for bearing through all these iterations to get to the final product.
I've tested all of the steps with my purposed changes above. Once you've reviewed and merged, I will do a final walkthrough. The rest of the grammatical and flow improvements I'll leave up to you.

mvincerx · 2019-09-24T20:42:41Z

@puneetse. Glad we can keep improving this and make it more practical. I'll restart testing it--got sidetracked earlier today-- so I'll push more revisions and let you know before EOB.

mvincerx · 2019-09-24T22:02:52Z

FYI: I resolved errors when running docker build commands. I validated docker commands --but had one minor error in Example 3 running the Python repl. Let's discuss Wed.

mvincerx · 2019-09-25T23:40:52Z

@puneetse, please see emails.

mvincerx

@puneetse. We're so close to ready on this. See Ln 133. When I validate, I still receive an error (see my comment). If you want to supply me with the output, just add a comment there. I'll take care of it.

mvincerx · 2019-10-01T21:54:07Z

+
+   .. code-block:: bash
+
+      docker run clearlinux/machine-learning-ui:31090 swupd info


@puneetse, this is the last remaining problem. I've run step 2 above twice now. On Step 3, I receive the same error message that was logged as this issue last March. If you can supply stdout for sample output, that'd be great.

@puneetse, please see my comment above. This is the last piece of data I need before we can merge.

The command above worked fine for me. It should look like the output of any other swupd info with 31090

docker run clearlinux/machine-learning-ui:31090 swupd info Distribution: Clear Linux OS Installed version: 31090 Version URL: https://cdn.download.clearlinux.org/update Content URL: https://cdn.download.clearlinux.org/update

Or, we can change it to this so that it matches the pattern in a previous section.:

docker run clearlinux/machine-learning-ui:31090 cat /usr/lib/os-release NAME="Clear Linux OS" VERSION=1 ID=clear-linux-os ID_LIKE=clear-linux-os VERSION_ID=31090 PRETTY_NAME="Clear Linux OS" ANSI_COLOR="1;35" HOME_URL="https://clearlinux.org" SUPPORT_URL="https://clearlinux.org" BUG_REPORT_URL="mailto:dev@lists.clearlinux.org" PRIVACY_POLICY_URL=http://www.intel.com/privacy

@puneetse. Please review latest commit.

- Add CLI steps to contextualize diff output. - Add Sphinx roles for clarity of command flags. - Apply edits for clarity of sequence and prior to output diffs. <michael.vincerra@intel.com>

MCamp859 requested a review from puneetse September 10, 2019 17:53

puneetse suggested changes Sep 10, 2019

View reviewed changes

puneetse reviewed Sep 10, 2019

View reviewed changes

mvincerx suggested changes Sep 11, 2019

View reviewed changes

MCamp859 force-pushed the mc-customize-app-cont branch 2 times, most recently from a91f2a1 to bf04cd0 Compare September 19, 2019 18:57

puneetse reviewed Sep 21, 2019

View reviewed changes

tpleavitt assigned mvincerx Sep 23, 2019

mvincerx force-pushed the mc-customize-app-cont branch from a3f3bef to f6d1422 Compare September 23, 2019 22:56

puneetse suggested changes Sep 24, 2019

View reviewed changes

puneetse reviewed Sep 24, 2019

View reviewed changes

Comment thread source/guides/maintenance/custom-app-container.rst Outdated

puneetse reviewed Sep 24, 2019

View reviewed changes

Comment thread source/guides/maintenance/custom-app-container.rst Outdated

puneetse reviewed Sep 24, 2019

View reviewed changes

Comment thread source/guides/maintenance/custom-app-container.rst Outdated

mvincerx reviewed Sep 26, 2019

View reviewed changes

Comment thread source/guides/maintenance/custom-app-container.rst Outdated

mvincerx reviewed Sep 26, 2019

View reviewed changes

Comment thread source/guides/maintenance/custom-app-container.rst Outdated

Comment thread source/guides/maintenance/custom-app-container.rst Outdated

mvincerx reviewed Oct 1, 2019

View reviewed changes

New doc: create custom application container image

fb7d0bc

- Add CLI steps to contextualize diff output. - Add Sphinx roles for clarity of command flags. - Apply edits for clarity of sequence and prior to output diffs. <michael.vincerra@intel.com>

mvincerx force-pushed the mc-customize-app-cont branch from ed0df67 to fb7d0bc Compare October 4, 2019 17:13

puneetse approved these changes Oct 4, 2019

View reviewed changes

mvincerx merged commit fbfe5f4 into clearlinux:master Oct 4, 2019

		to edit \|CL\| container base images for the following situations:


		* Customize an application image Dockerfile by adding a bundle

		@@ -0,0 +1,402 @@
		.. _custom-app-container:

		Create customized application container images

	Create customized application container images
	Modify existing application container images

	steps to add Tensorflow\* into the clearlinux/python container at runtime.
	steps to add Tensorflow\* into a clearlinux/python container at runtime, as an example.

	$ docker run -it --rm clearlinux/python
	docker run -it --rm clearlinux/python

	#. In a third console, save the container as clearlinux/python:tensorflow_added.
	#. In a third console, save the container with a new tag.



		Customize an application image at runtime
		=========================================


		.. code-block:: bash

		docker run clearlinux/machine-learning-ui:31090 swupd info

		Dockerfile:


		.. contents::

Conversation

MCamp859 commented Sep 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

puneetse left a comment

Choose a reason for hiding this comment

Uh oh!

mvincerx left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MCamp859 commented Sep 19, 2019

Uh oh!

mvincerx commented Sep 20, 2019

Uh oh!

mvincerx commented Sep 20, 2019

Uh oh!

mvincerx commented Sep 20, 2019

Uh oh!

mvincerx commented Sep 21, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!