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

chore: reconfigure docker-compose networks #6068

Merged
merged 1 commit into from
Jan 31, 2020

Conversation

aldeed
Copy link
Contributor

@aldeed aldeed commented Jan 30, 2020

There are 5 related PRs that should be tested together:
reactioncommerce/example-storefront#653
reactioncommerce/reaction-hydra#46
reactioncommerce/reaction-admin#194
reactioncommerce/reaction-identity#23
#6068

All of these ensure that docker-compose defines exactly one external network named reaction.localhost and update all services and ENV to use that one. This is being done to cut down on confusion caused by having various arbitrary networks. This affects only local development using docker-compose.

The only downside we're aware of is that project authors need to be more careful about giving Reaction services unique names (e.g., can't call them all web).

One upside in addition to being generally less confusing is that the primary internal hostname for the API is now just api.reaction.localhost rather than the more confusing api.api.reaction.localhost.

Testing

  1. Switch to the PR branches of all 5 projects linked above.
  2. Copy .env.example to .env in each project, completely replacing .env.
  3. Stop all Docker containers.
  4. docker system prune
  5. Delete the docker-compose.override.yml file from each of the project directories. It doesn't technically matter whether you leave the override in place, but everything will start faster if you don't.
  6. In the platform directory, make start.
  7. Once all services are running, check all service logs for errors, and do some basic smoke tests such as logging in to both admin and storefront.

Copy link
Member

@kieckhafer kieckhafer left a comment

Choose a reason for hiding this comment

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

A few issues I've come across:

I've had to manually create the following two networks when running make start:
auth.reaction.localhost
reaction.localhost

thrown from start-reaction-hydra:
Network auth.reaction.localhost declared as external, but could not be found. Please create the network manually using docker network create auth.reaction.localhost and try again.

thrown from start-reaction-identity:
Network reaction.localhost declared as external, but could not be found. Please create the network manually usingdocker network create reaction.localhost and try again.

Should this all be done automatically at this point?


There is some sort of race condition we've been seeing where identity needs api to be started, so it fails starting the first time, but works fine after api is started and identity is restarted. This isn't new to this ticket, but just mentioning it here too.


Admin does not start correctly because of the above issue, but contraty to identity, which starts as expected after api is running, when i try to restart admin at this point i get another missing address that wasn't thrown on the initial `make start:

FetchError: request to http://hydra.reaction.localhost:4445/clients/reaction-admin failed, reason: getaddrinfo ENOTFOUND hydra.reaction.localhost
    at ClientRequest.<anonymous> (/usr/local/src/app/programs/server/npm/node_modules/node-fetch/lib/index.js:1444:11)
    at ClientRequest.emit (events.js:210:5)
    at ClientRequest.EventEmitter.emit (domain.js:475:20)
    at Socket.socketErrorListener (_http_client.js:406:9)
    at Socket.emit (events.js:210:5)
    at Socket.EventEmitter.emit (domain.js:475:20)
    at emitErrorNT (internal/streams/destroy.js:92:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
    at processTicksAndRejections (internal/process/task_queues.js:80:21)
 => awaited here:
    at Function.Promise.await (/usr/local/src/app/programs/server/npm/node_modules/meteor/promise/node_modules/meteor-promise/promise_server.js:56:12)
    at imports/plugins/core/core/server/startup/oauth.js:40:26
    at /usr/local/src/app/programs/server/npm/node_modules/meteor/promise/node_modules/meteor-promise/fiber_pool.js:43:40
 => awaited here:
    at Function.Promise.await (/usr/local/src/app/programs/server/npm/node_modules/meteor/promise/node_modules/meteor-promise/promise_server.js:56:12)
    at startup (imports/plugins/core/core/server/startup/index.js:41:3)
    at Function.time (/usr/local/src/app/programs/server/profile.js:273:30)
    at /usr/local/src/app/programs/server/boot.js:412:15
    at /usr/local/src/app/programs/server/boot.js:462:7
    at Function.run (/usr/local/src/app/programs/server/profile.js:280:14)
    at /usr/local/src/app/programs/server/boot.js:460:13 {
  message: 'request to http://hydra.reaction.localhost:4445/clients/reaction-admin failed, reason: getaddrinfo ENOTFOUND hydra.reaction.localhost',
  type: 'system',
  errno: 'ENOTFOUND',
  code: 'ENOTFOUND'
}

@aldeed
Copy link
Contributor Author

aldeed commented Jan 31, 2020

@kieckhafer Can you post your Hydra logs? It might be the same issue @manueldelreal saw.

The race condition is separate from this issue. The network creation is expected, but I agree make start should do it. I'll see if that's easy to add.

@kieckhafer
Copy link
Member

kieckhafer commented Jan 31, 2020

@aldeed there are no errors in the hydra logs.

Hydra Logs:

Config file not found because "Config File ".hydra" Not Found in "[/]""
time="2020-01-31T18:05:39Z" level=info msg="No tracer configured - skipping tracing setup"
time="2020-01-31T18:05:39Z" level=info msg="Establishing connection with SQL database backend" dsn="postgres://*:*@postgres.reaction.localhost:5432/hydra?sslmode=disable"
time="2020-01-31T18:05:39Z" level=info msg="Successfully connected to SQL database backend" dsn="postgres://*:*@postgres.reaction.localhost:5432/hydra?sslmode=disable"
time="2020-01-31T18:05:39Z" level=fatal msg="Could not ensure that signing keys for \"hydra.openid.id-token\" exists. This can happen if you forget to run \"hydra migrate sql\", set the wrong \"secrets.system\" or forget to set \"secrets.system\" entirely." error="pq: relation \"hydra_jwk\" does not exist"

I do see a similar error from identity in example-storefront though that I didn't notice before, and example-storefront also doesn't start:

FetchError: request to http://hydra.reaction.localhost:4445/clients/example-storefront failed, reason: getaddrinfo ENOTFOUND hydra.reaction.localhost hydra.reaction.localhost:4445
    at ClientRequest.<anonymous> (/usr/local/src/app/node_modules/node-fetch/index.js:133:11)
    at ClientRequest.emit (events.js:198:13)
    at Socket.socketErrorListener (_http_client.js:392:9)
    at Socket.emit (events.js:198:13)
    at emitErrorNT (internal/streams/destroy.js:91:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
    at process._tickCallback (internal/process/next_tick.js:63:19)

I ran docker network create hydra.reaction.localhost just to see if it worked, and that did not help.

@aldeed
Copy link
Contributor Author

aldeed commented Jan 31, 2020

Fixed the Hydra startup issue in reactioncommerce/reaction-hydra@929119d

@kieckhafer
Copy link
Member

With the hydra update, this works as expected for me. There are two issues which I consider unrelated to this PR and I'm OK for merging all of these without resolving these first.

  1. there is that race condition we keep seeing where identity starts up and then fails because api isn't started. we need to figure that out before v3.0.0 but not in this PR
  2. the initial user isn't getting added to the systems-manager group like it's supposed to be, meaning you can't create a shop once you're logged in. I'll look into this in another issue, but for now if you want to test, go into the mongodb manually and just add the system-manager groupID to your accounts group array

@manueldelreal
Copy link
Member

With the hydra update, this works as expected for me. There are two issues which I consider unrelated to this PR and I'm OK for merging all of these without resolving these first.

  1. there is that race condition we keep seeing where identity starts up and then fails because api isn't started. we need to figure that out before v3.0.0 but not in this PR
  2. the initial user isn't getting added to the systems-manager group like it's supposed to be, meaning you can't create a shop once you're logged in. I'll look into this in another issue, but for now if you want to test, go into the mongodb manually and just add the system-manager groupID to your accounts group array

For me it's not just identity, it's also admin and example-storefront. startup for these three is way faster than the startup for the api container so they error out when they try to reach the api and it's still coming up, but this is for another ticket.

Copy link
Member

@manueldelreal manueldelreal left a comment

Choose a reason for hiding this comment

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

:shipit:

@aldeed aldeed merged commit 134bd17 into release-3.0.0 Jan 31, 2020
@aldeed aldeed deleted the chore-dc-network-rename branch January 31, 2020 20:52
rosshadden added a commit to reactioncommerce/federated-gateway that referenced this pull request Feb 3, 2020
rosshadden added a commit to reactioncommerce/federated-gateway that referenced this pull request Feb 3, 2020
@kieckhafer kieckhafer mentioned this pull request Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants