Skip to content

feat: support official docker images#54

Merged
varl merged 15 commits intomasterfrom
align-with-official-images
May 27, 2019
Merged

feat: support official docker images#54
varl merged 15 commits intomasterfrom
align-with-official-images

Conversation

@varl
Copy link
Copy Markdown
Contributor

@varl varl commented May 24, 2019

Started with the makeDockerImage resolutions for supporting channels and versions from https://hub.docker.com/u/dhis2.

  • Adds support for official dhis2 docker images in different channels (core, core-dev, core-canary)
  • Adds --db-version <string>
  • Adds --variant <array>
  • Adds --channel <string>
  • Adds tests for configuration merging and generating docker image names

Combos

# 2.32 works for both dhis2 and db version
d2 cluster up 2.32 \
  --channel canary

# custom images require explicit context
d2 cluster up custom \
  --image amcgee/dhis2-core:dev-alpine \
  --db-version 2.32 \ 
  --seed

# sometimes the dhis2 and db versions do not match
d2 cluster up stable \
  --dhis2-version 2.31.4 \
  --db-version 2.30 \
  --seed

# dhis2 version can be used for db version if it matches
d2 cluster up 232-dev \
  --channel dev \
  --dhis2-version 2.32 \
  --seed

Save per cluster configuration

To allow for subsequent startups without all the switches we need a mechanism (e.g. #53) to save the configuration first passed to the up command.

So first time the cluster is created it stores the config in e.g. ~/.cache/d2/d2-cluster/dev/configCache.json

This enables:

d2 cluster up dev \
  --custom-context \
  --channel dev \
  --dhis2-version master \
  --db-version dev \
  --seed

d2 cluster down dev

d2 cluster up dev
# will use the `configCache`

Mentioning it here to note that implementing that is out of scope for this PR, but there are plans to do it.

@varl varl requested a review from amcgee May 24, 2019 15:35
Comment thread packages/cluster/src/commands/up.js
@varl varl requested a review from vilkg May 24, 2019 16:14
const resolvedContext =
customContext || cluster.customContext || defaults.customContext
const resolvedContextPath = resolvedContext ? `/${name}` : ''
const runtime = makeEnvironment(argv, {}, cluster)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The second empty object is going to be replaced with the per-cluster cached configuration. The cluster object is the default configuration from ~/.config/d2/config.js.

amcgee
amcgee previously approved these changes May 27, 2019
Copy link
Copy Markdown
Contributor

@amcgee amcgee left a comment

Choose a reason for hiding this comment

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

This looks really great!! I made a couple minor comments, but happy with it as is as well. Also yay tests!!

Comment thread packages/cluster/src/commands/up.js
Comment thread packages/cluster/tests/substitute-string-keys.js Outdated
@varl
Copy link
Copy Markdown
Contributor Author

varl commented May 27, 2019

@amcgee did three changes:

  1. split the makeEnvironment in twain; resolveConfiguration and makeEnvironment.
  2. added default where important (channel and port) to the description.
  3. changed the --variant switch to accept a string, so users would have to say --variant tomcat-alpine, this works around the ordering problem with an array switch.

Comment thread packages/cluster/src/commands/up.js Outdated
type: 'string',
},
channel: {
desc: 'Set the release channel to use (default: stable)',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this use the same defaults logic as the other properties? So basically that would mean removing the if !substitutes.channel check from makeDockerImage and adding channel: 'stable' to defaults ...?

Copy link
Copy Markdown
Contributor Author

@varl varl May 27, 2019

Choose a reason for hiding this comment

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

Sure, I can change the defaults.channel to stable and use the same logic. 👍

I cannot remove the !substitutes.channel check though since the makeDockerImage function doesn't do the configuration resolution itself, and if the function is used with a substitution object which doesn't pass in the channel, it won't get replaced with an empty string. The tests catch this situation and started to error out.

It would have been a bit simpler if the stable repo was called core-stable and not just core, but I'd rather have a little bit of extra complexity here and a nicer repo name than the other way around.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

@amcgee amcgee left a comment

Choose a reason for hiding this comment

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

This is great! Tested locally, working like a charm. And now that @vilkg has pushed stable releases to dhis2/core the world is starting to make sense :-D

@varl varl merged commit 8e2a6da into master May 27, 2019
@varl varl deleted the align-with-official-images branch May 27, 2019 12:53
dhis2-bot pushed a commit that referenced this pull request May 27, 2019
# [1.2.0](v1.1.0...v1.2.0) (2019-05-27)

### Features

* support official docker images ([#54](#54)) ([8e2a6da](8e2a6da))
@dhis2-bot
Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants