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.

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

PAYARA-4099 Allow Creation of Temp Nodes #4260

Merged
merged 14 commits into from Nov 11, 2019

Conversation

@Pandrex247
Copy link
Member

Pandrex247 commented Oct 8, 2019

Description

This is a new feature intended for use with Docker/Kubernetes to more easily allow autoscaling.

Previously, autoscaling could only be done in a limited capacity, and very messily to boot, due to the restriction of requiring a node to be mapped to each instance for the DAS to talk to. This PR introduces the concept of Temp (temporary) nodes, which are similar to config nodes, but only persist as long as they have instances assigned to them. As demonstrated in the updated Docker Node image, this allows containers to create their own nodes and instances and register themselves to the DAS (autogenerating names as required).

Temp nodes and instances created upon them have some behaviour that differentiate themselves from normal Config nodes:

  • When an instance created on a temp node is stopped, it is automatically deleted.
  • When Payara Server detects that a temp node has no more instances assigned to it, the node is deleted.
    • Payara Server performs this check upon instance stop/deletion, and when shutting down. This is to avoid unnecessary locking or race conditions caused by a periodic reaper service: creation of nodes and instances are done separately and can be interrupted.
  • Temp nodes do not show up on the admin console, and are read only.
    • They do however still show up if you look in the REST management console, or inspect the dotted name tree / domain.xml - I'm not aware of any way around this.

Testing

New tests

JsonRequestConstructorTest.java updated with custom config name (rather than using default).
Otherwise none - any test requires environment setup (Docker, multiple VMs etc.).

Testing Performed

All testing performed with DAS on a Windows machine, and Docker containers on a Linux VM (so I can use host network mode).

Creating an instance using Docker REST API:

  • Setup DAS on Windows Host:
    • Start clean domain
    • change-admin-password
    • enable-secure-admin
    • restart-domain
  • Create Docker container:
    • curl -X POST -H 'Accept: application/json' -H 'Content-Type: application/json' -i 'http://192.168.150.141:2376/containers/create' --data '{ "Image": "payara/server-node:latest", "HostConfig": { "Mounts": [ { "Type": "bind", "Source": "/home/andrew/DockerTest/passwordfile.txt", "Target": "/opt/payara/passwords/passwordfile.txt", "ReadOnly": true } ], "NetworkMode": "host" }, "Env": [ "PAYARA_DAS_HOST=192.168.253.1" ] }'
  • Repeated with various bits of info added or missing (e.g. instance name provided, config name provided, node name provided)

Restarting a container that was stopped manually:
If given an instance name, will attempt to start existing instance, if given no info, will create new instance and node from scratch.

Temp Nodes Deletion

  • Stop a running instance on a temp node from DAS > instance stopped and deleted, plus node deleted (checked via REST Management interface)
  • Delete a stopped instance on a temp node from DAS (instance stopped without deletion by stopping container) > instance and node deleted.
  • Delete a stopped instance on a temp node with more than one instance > instance deleted, node left alone.
  • Temp nodes deleted on DAS restart:
    • Stop containers to stop instances without deletion
    • Restart DAS > instances and node should be gone upon restart.

Test suites executed

  • Quicklook
  • MicroProfile TCK Runners

Testing Environment

Zulu JDK8u222, OpenSUSE Tumbleweed Linux, Maven 3.6.0.
Zulu JDK8u222, Windows 10, Maven 3.6.0.
Docker 19.03.1, API 1.40

Documentation

payara/Payara-Server-Documentation#632

Notes for Reviewers

You'll need to build the Docker image and then specify that it's the image you want to use in your containers. Depending on your environment you can build the image as a part of the normal Maven build by adding the profile -PBuildDockerImages. Otherwise build the image manually using Docker CLI.

@Pandrex247

This comment has been minimized.

Copy link
Member Author

Pandrex247 commented Oct 8, 2019

Jenkins test please

@Pandrex247 Pandrex247 force-pushed the Pandrex247:PAYARA-4099-Rebase branch from 95d7bbe to 8744d3b Oct 10, 2019
@Pandrex247 Pandrex247 dismissed Cousjava’s stale review Oct 16, 2019

Changes made

@dmatej dmatej self-requested a review Oct 29, 2019
@dmatej
dmatej approved these changes Oct 29, 2019
Copy link
Contributor

dmatej left a comment

It works, but I found only minor issues in sources. Also it can be rebased on current master without problems.

### Functions ###
function checkAndCreateNewNodeIfRequired {
if [ -z ${PAYARA_NODE_NAME} ]; then

This comment has been minimized.

Copy link
@dmatej

dmatej Oct 29, 2019

Contributor

Missing quotation marks

This comment has been minimized.

Copy link
@Pandrex247

Pandrex247 Oct 29, 2019

Author Member

Are they necessary? I haven't got them anywhere else and it works fine from my testing.

This comment has been minimized.

Copy link
@dmatej

dmatej Oct 29, 2019

Contributor

https://stackoverflow.com/questions/51112152/how-to-accept-a-quoted-sentence-as-argument-in-shell-script - I didn't try, but I expect that if I would set the property with spaces inside, the script would crash.

echo "./payara5/bin/asadmin -I false -T -a -H ${PAYARA_DAS_HOST} -p ${PAYARA_DAS_PORT} -W ${PAYARA_PASSWORD_FILE} create-local-instance --node ${PAYARA_NODE_NAME} --dockernode true ${PAYARA_INSTANCE_NAME}"
PAYARA_INSTANCE_NAME="$(./payara5/bin/asadmin -I false -T -a -H ${PAYARA_DAS_HOST} -p ${PAYARA_DAS_PORT} -W ${PAYARA_PASSWORD_FILE} create-local-instance --node ${PAYARA_NODE_NAME} --dockernode true ${PAYARA_INSTANCE_NAME})"
if [ -z ${PAYARA_CONFIG_NAME} ]; then
echo "./payara5/bin/asadmin -I false -T -a -H ${PAYARA_DAS_HOST} -p ${PAYARA_DAS_PORT} -W ${PAYARA_PASSWORD_FILE} create-local-instance --node ${PAYARA_NODE_NAME} --dockernode true --ip ${DOCKER_CONTAINER_IP} ${PAYARA_INSTANCE_NAME} "

This comment has been minimized.

Copy link
@dmatej

dmatej Oct 29, 2019

Contributor

To debug commands you can also put this after the first line of the bash script:
set -x
Or put the command to a variable and then use echo and eval.


if [ ! -z ${EXISTS} ]; then
if [ ! -z ${INSTANCE_EXISTS} ]; then

This comment has been minimized.

Copy link
@dmatej

dmatej Oct 29, 2019

Contributor

Missing quotation marks

@Pandrex247 Pandrex247 force-pushed the Pandrex247:PAYARA-4099-Rebase branch 2 times, most recently from c41f810 to 3175b8d Oct 29, 2019
@dmatej dmatej self-requested a review Oct 31, 2019
Copy link
Contributor

dmatej left a comment

I found a bug - it is not possible to uncheck the checkbox Use TLS once it was checked and saved - after Save it is checked again.
image

Also Docker part is shown only after the Save button was pressed.
EDIT: was caused by an old woodstock in my maven cache.

@dmatej dmatej self-requested a review Nov 5, 2019
@dmatej
dmatej approved these changes Nov 5, 2019
@dmatej

This comment has been minimized.

Copy link
Contributor

dmatej commented Nov 5, 2019

Jenkins test please

@Pandrex247 Pandrex247 merged commit 648a050 into payara:master Nov 11, 2019
58 checks passed
58 checks passed
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/concurrent/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ha/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details
@Pandrex247 Pandrex247 deleted the Pandrex247:PAYARA-4099-Rebase branch Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.