Docker Improvements #489

gartens · 2024-04-12T09:10:48Z

Summary

Various improvements to the Docker subsystem, such as:

Getting rid of Map.of in favor of properly annotated records/classes
Handshakes are now identified by an id, not the hostname
Better error handling by introducing DockerUserException which carries an appropriate HTTP status code for the response.
Change the schema of the endpoints to contain the ids, be a bit more CRUD like and use HTTP verbs like DELETE etc.
Various other small improvements

See #94 for the UI changes.

This is done first for the Handshakes, by introducing a nice HandshakeInfo that also holds a DockerHost. This commit also adds a DockerUserException, the User part is to say, that it is something nice that can be shown to users (i.e. in the frontend).

When restoring instances on startup, and this connection fails, instances are added in the "NEW" state, without an UUID. So check for that.

This allows status codes to be communicated with the Exception. Default status is 400 (Bad Request).

While here also add a helper for a common task: collect the Maps (soon to be proper records) for each DockerInstance.

The frontend now updates the entire state with getDockerInstance, which is also improved in this commit.

This gets rid of a bunch Map<String, Objects> everywhere.

They now all start with /docker/ and somewhat look like CRUD endpoints. Some endpoints also use HTTP verbs like DELETE instead of everything being just GET/POST. Also add a new docker/handshakes endpoint that returns all handshakes from the HandshakeManager.

…, Object>

…r is cancelled

The only regression is the change of hostnames, but instead of doing a handshake like before, one of the upcoming commits will change this to copy/move the encryption keys, which should always work (because it is the same instance). Maybe it will also allow to change the ports...

Moving the opening of the socket after the certificate handling makes it even less likely we open it without needing it.

This ensures that we change the state, which allows waiting threads to make progress.

With this we can reduce the visibility of isConnected a little bit

datomo

Thank you @gartens, this looks very nice and clean.
With the minor things and one final reformat out of the way this should be good to go.

core/src/main/java/org/polypheny/db/docker/DockerInstance.java

core/src/main/java/org/polypheny/db/docker/PolyphenyHandshakeClient.java

core/src/main/java/org/polypheny/db/docker/models/AutoDockerResult.java

core/src/main/java/org/polypheny/db/docker/models/AutoDockerStatus.java

core/src/main/java/org/polypheny/db/docker/DockerContainer.java

core/src/main/java/org/polypheny/db/docker/DockerInstance.java

core/src/main/java/org/polypheny/db/docker/PolyphenyCertificateManager.java

vogti

Thank you, @gartens, for this PR!

gartens added 30 commits April 11, 2024 16:44

Add a record to hold the default registry

4b0d7f8

AutoDocker gets proper records

db45415

Use pattern variables and fix typo in PolyphenyHandshakeClient

7301bb9

Serialize fields which are null for AddDockerResponse

5aade10

Fix NullPointerException

2ea170b

When restoring instances on startup, and this connection fails, instances are added in the "NEW" state, without an UUID. So check for that.

Use an id instead of the hostname to identify handshakes

4e867d6

Add a status field to DockerUserException

ec41d22

This allows status codes to be communicated with the Exception. Default status is 400 (Bad Request).

Improve removeDockerInstance

e106d03

While here also add a helper for a common task: collect the Maps (soon to be proper records) for each DockerInstance.

Skip copying dockerInstances

d78b11e

Improve reconnect API call

04eb8e5

Format

5f92730

Remove testDockerConnection

aa4d643

The frontend now updates the entire state with getDockerInstance, which is also improved in this commit.

Add DockerInstanceInfo record

2648085

This gets rid of a bunch Map<String, Objects> everywhere.

Chain ctx calls

8622e93

Remove unused helper class

e803ef6

Remove unused imports and another now unused class

c3ad96e

Adjust visibility of ensureHandshakeIsRunning

da5a35e

Fix typo

e0eef74

Fix comment about compatibility of messages

9eadb84

Rename Docker API endpoints

de1a8af

They now all start with /docker/ and somewhat look like CRUD endpoints. Some endpoints also use HTTP verbs like DELETE instead of everything being just GET/POST. Also add a new docker/handshakes endpoint that returns all handshakes from the HandshakeManager.

C in CRUD is for Create

3843548

Use a new CreateDockerRequest record to get rid of another Map<String…

d8e1b0f

…, Object>

Use names that are a bit more CRUD

a09a8ee

Change updateDockerInstance from POST to PATCH

81cd832

Separate cancel and delete endpoints

2347e8b

Add more locking, so that the handshake executes its success action o…

b4bc944

…r is cancelled

Remove unused imports

83674e8

Remove superfluous empty line

bb348dc

gartens added 15 commits April 11, 2024 16:44

Merge in various improvements from another branch

4150b0a

Check that the instance is not NEW before modifying status

9a492fa

Rename field for consistency

3c3566e

Make sure the socket gets closed in case of an error

5174887

Moving the opening of the socket after the certificate handling makes it even less likely we open it without needing it.

Catch exceptions in the success callback

8a58107

This ensures that we change the state, which allows waiting threads to make progress.

Properly report AutoDocker handshake errors to the user

8658325

Have Notebooks use the DockerInstanceInfo

7042e7f

With this we can reduce the visibility of isConnected a little bit

Ensure that registry is never null

b3e7627

Allow handshakes to be "restarted"

55f588d

No need for a stacktrace

96cb434

Bring back the ping call, in hope to make the getInfo method faster

d5568c1

Fix issue from rebase

17498c1

Set names for threads

6076b07

Bump bouncycastle

d630613

Move Polypheny UUID generation into its own method

83ae441

gartens mentioned this pull request Apr 12, 2024

Various improvements to the Docker configuration polypheny/Polypheny-UI#94

Merged

Bump version numbers in licensee task

51193cd

datomo requested changes Apr 14, 2024

View reviewed changes

gartens added 6 commits April 15, 2024 09:32

Reformat and update .editorconfig

50fd53b

Display a name, not integer, for the user to choose a Docker instance

1231417

Update the catalog when the Docker config changes

f976be9

Remove dynamic adapter settings

8621726

Only list active handshakes (i.e. not cancelled and not successful)

a8502cb

Remove commented code

4514460

vogti requested changes Apr 17, 2024

View reviewed changes

core/src/main/java/org/polypheny/db/docker/DockerContainer.java Outdated Show resolved Hide resolved

core/src/main/java/org/polypheny/db/docker/DockerInstance.java Show resolved Hide resolved

core/src/main/java/org/polypheny/db/docker/PolyphenyCertificateManager.java Show resolved Hide resolved

Fix typo

a71d7c2

vogti approved these changes Apr 17, 2024

View reviewed changes

datomo approved these changes Apr 17, 2024

View reviewed changes

vogti merged commit 07138c6 into master Apr 18, 2024
38 checks passed

vogti deleted the docker-improvements branch April 18, 2024 05:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docker Improvements #489

Docker Improvements #489

gartens commented Apr 12, 2024 •

edited

Loading

datomo left a comment

vogti left a comment

Docker Improvements #489

Docker Improvements #489

Conversation

gartens commented Apr 12, 2024 • edited Loading

Summary

datomo left a comment

Choose a reason for hiding this comment

vogti left a comment

Choose a reason for hiding this comment

gartens commented Apr 12, 2024 •

edited

Loading