Skip to content

Conversation

ykotecha
Copy link
Contributor

Please review SOA, OSB and BPM docker files. Thanks.

Copy link
Contributor

@Djelibeybi Djelibeybi left a comment

Choose a reason for hiding this comment

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

You are relying on the Docker Compose sample to switch between the SOA/BPM and Admin/Managed server types, but you don't document how to do it manually for users who don't use Compose or want to use an alternative orchestration system like Kubernetes.

Further, it relies on manually calling a specific command. You should rather accept a runtime environment variable (e.g. soaas, soams, etc) and a single script called by CMD should configure and/or start the right server.

Then, the compose file just needs to set the right ENV variable.

#
# Pull base image
# ---------------
FROM container-registry.oracle.com/java/serverjre:8u131
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be changed to FROM oracle/serverjre:8.

Copy link
Contributor

Choose a reason for hiding this comment

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

I did not see 8 in container-registry - that is why used this so that we can pull an already existing certified image. Can be changed easily, but local tags would need to be done then.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is definitely an 8 in the Container Registry, but either way, you need to use oracle/serverjre:8 for Dockerfiles in this repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

FIXED. We are now inheriting from the FMW Infra Image.

#
# Environment variables required for this build (do NOT change)
# -------------------------------------------------------------
USER root
Copy link
Contributor

Choose a reason for hiding this comment

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

This line isn't necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

FIXED.

# Copy Domain creation scripts
#
USER root
COPY container-scripts/* /u01/oracle/dockertools/
Copy link
Contributor

Choose a reason for hiding this comment

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

Move lines 84-85 to after line 72 so that you don't have to switch between root and oracle.

Copy link
Contributor

Choose a reason for hiding this comment

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

FIXED.

#
# Define default command to start bash.
#
USER oracle
Copy link
Contributor

Choose a reason for hiding this comment

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

If lines 84-85 are moved, you don't need to switch back to oracle here.

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason for the switch was we found that everytime there is a change in one of our scripts an entire reinstall was done as the layer was marked as changed - so the cache was invalidated. This resulted in a really long build time for each minor change in the script. By doing this we get the benefit of caching. The developer will face this too, which could be an issue. The reason for the change was a much better user experience.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just as a reference point if we made a change to container-scripts directory which is primarily where all code is, if we did not split it up a build takes around 900 seconds, whereas with this shift it comes down to about 100 seconds.

Copy link
Contributor

Choose a reason for hiding this comment

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

Once your PR is merged, your scripts wouldn't change, so this is not impacted. Besides, making an efficient image with the least amount of layers is better than jumping between users and layers.

Copy link
Contributor

Choose a reason for hiding this comment

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

We had exactly this before - and it was just becoming difficult to iterate locally. I assume after merging the developer is likely to change the scripts - so they will face the issue if they do this. It is a simple change so it can be done - but I wanted to make sure you understand the rationale.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why would a developer be likely to change these scripts after merge? That bit I don't understand. These scripts should be production-ready and not require any modification after merging.

Copy link
Contributor

Choose a reason for hiding this comment

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

FIXED.

if you have an internal proxy - ensure that the proxy
related env variables are set
http_proxy, https_proxy, no_proxy
- Pull the base image into your local docker cache for builds.
Copy link
Contributor

Choose a reason for hiding this comment

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

The preferred mechanism for manually building images is to manually build the base image. See the WebLogic Dockerfiles for examples on how to either manually build the oracle/serverjre image or pull it from the Oracle Container Registry or Docker Store and tag it accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

NOTED.

You will need to provide your login credentials.

```sh
docker pull container-registry.oracle.com/java/serverjre:8u131
Copy link
Contributor

Choose a reason for hiding this comment

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

Specifying 8u131 is too specific, because it will lock users into a JRE version that won't be updated. You should specify 8 instead so that when the JRE is updated, any subsequent builds of SOA/BPM will use the updated JRE as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I did not see 8 in container-registry - that is why used this so that we can pull an already existing certified image.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's there. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Easy change.

Copy link
Contributor

Choose a reason for hiding this comment

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

FIXED. No longer applicable as we use the FMWInfra Image.

export DC_ORCL_OEM_PORT=5500
export DC_ORCL_SID=soadb
export DC_ORCL_PDB=soapdb
export DC_ORCL_SYSPWD=welcome1
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't allow hard-coded passwords anywhere. This needs to be changed to either accept a password or dynamically generate one.

Copy link
Contributor

Choose a reason for hiding this comment

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

We support auto password generation - we can remove this.

Copy link
Contributor

Choose a reason for hiding this comment

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

FIXED.

Copy link
Member

@mriccell mriccell left a comment

Choose a reason for hiding this comment

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

Extend the FMW Infra image instead of reinstalling the same binaries!

#
# Pull base image
# ---------------
FROM container-registry.oracle.com/java/serverjre:8u131
Copy link
Member

Choose a reason for hiding this comment

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

Why are you not extending the FMW Infrastructure image, taking advantage of the layering instead of reinstalling the same binaries here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Our Quickstart installers already include FMW infrastructure in them - so they directly do the install. Otherwise it will be a double install.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have alternative installers that don't include FMW infrastructure? If so, can you use those instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but that is a lot of separate installers - this was built purely on the request of customers as they wanted to get done in a single download.

Copy link
Contributor

Choose a reason for hiding this comment

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

Customers are going to build this once and then distribute internally. The images we provide on OCR and other registries will be built off the FMW Infrastructure image. This needs to change to inherit from FMW Infrastructure, because that's exactly what it was designed for: to be the base for SOA/BPM/OSB.

Copy link
Contributor

Choose a reason for hiding this comment

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

FIXED. We are now inheriting from the FMW Infra Image. We need the official coordinates to use in the FROM tag. Please provide that. Currently using
FROM middleware/weblogic:12.2.1.2-infrastructure

Copy link
Member

@mriccell mriccell left a comment

Choose a reason for hiding this comment

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

Is this meant to run in a multi host env? If so how do you address the mapping of the domain to a host dir? Maybe the mapping should be to a data container that is in the overlay folder.

#
# RCU Common password for all schemas + Prefix Names
#
export DC_RCU_SCHPWD=welcome1
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, passwords can not be hardcoded

Copy link
Contributor

Choose a reason for hiding this comment

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

FIXED.

related env variables are set
http_proxy, https_proxy, no_proxy
- Pull the base image into your local docker cache for builds.
You will need to provide your login credentials.
Copy link
Member

Choose a reason for hiding this comment

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

Can you provide more details about what credentials need to be provided?

Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be required if the primary documentation states that the oracle/serverjre:8 is manually built using the Dockerfile from the repo, instead of being pulled from OCR or Docker Store.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is more an internal thing here - though users may face it too. Basically the proxy values not being set results in very slow downloads as it contacts the proxy for internal URLs too. This is again generic stuff that we found along the way as we were doing work - and documented it for others that may run into the same issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please make sure the Dockerfiles and scripts here work for the non-employee user.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes they will. OracleSOABPM renamed to OracleSOASuite.

docker logs -f soaas
```
- Verify Admin Server
http://host.acme.com:7001/console (weblogic/welcome1)
Copy link
Member

Choose a reason for hiding this comment

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

How do you set the host to be always host.acme.com? How is the admin password becoming welcome1 if it is auto-generated. You want to make in the dockerfile the ADMIN_PASSWORD an ENV and allow the user if they do not want the autop generated password to over-write with a -e option.

Copy link
Contributor

Choose a reason for hiding this comment

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

These are just examples for the user. They need to substitute their actual values (of host and username/password etc)

Copy link
Contributor

Choose a reason for hiding this comment

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

If they're examples, then the documentation needs to be more explicit about that and what the user needs to replace them with.

Copy link
Contributor

Choose a reason for hiding this comment

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

We will just remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

FIXED.

Copy link
Contributor

@brunoborges brunoborges left a comment

Choose a reason for hiding this comment

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

Please rename folder from OracleSOABPM to OracleSOASuite.

related env variables are set
http_proxy, https_proxy, no_proxy
- Pull the base image into your local docker cache for builds.
You will need to provide your login credentials.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make sure the Dockerfiles and scripts here work for the non-employee user.

Copy link
Contributor

@Djelibeybi Djelibeybi left a comment

Choose a reason for hiding this comment

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

Great work, thanks. Just need @mriccell and @brunoborges to sign off too.

Copy link
Contributor

@brunoborges brunoborges left a comment

Choose a reason for hiding this comment

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

Suite! (pun intended)...
LGTM

@Djelibeybi Djelibeybi merged commit 5deb063 into oracle:master Jul 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants