feat: decouple tag, name, and version #46

varl · 2019-05-14T06:40:06Z

In preparation for #45 this PR experiments with the consequences of decoupling the version, tag, and name of a cluster.

varl · 2019-05-15T11:51:08Z

This decouples the name of the cluster from the version and image it is running.

The name is the name of the cluster, and the context path where the application will be accessible.

repo and tag refers to the Docker repo and tag.

# run a specific docker repo and tag on the customdev context
d2 cluster up customdev --repo amcgee/dhis2-core --tag dev-latest
# => http://localhost:8080/customdev



# set the DHIS2 version to use to find the right DB version dump
d2 cluster up dev --ver=dev --seed
# => http://localhost:8080/dev



# use defaults from `~/.config/d2` config
cat ~/.config/d2/config.js

module.exports = {
    cluster: {
        dockerComposeRepository: 'https://github.com/varl/dhis2-backend/archive/master.tar.gz',
        demoDatabaseURL: 'https://github.com/dhis2/dhis2-demo-db/blob/master/sierra-leone/{version}/dhis2-db-sierra-leone.sql.gz?raw=true',
        tag: 'dev-latest-alpine',
        repo: 'dhis2/core',
        ver: 'dev',
    }
}

d2 cluster up mydev
# => http://localhost:8080/mydev

Still a work in progress.

varl · 2019-05-15T11:53:46Z

Depends on https://github.com/varl/dhis2-backend as there are some docker-compose.yml changes needed to be able to decouple everything.

varl · 2019-05-16T15:59:02Z

New config file:

module.exports = {
    cluster: {
        dockerComposeRepository: 'https://github.com/varl/dhis2-backend/archive/master.tar.gz',
        demoDatabaseURL: 'https://github.com/dhis2/dhis2-demo-db/blob/master/sierra-leone/{version}/dhis2-db-sierra-leone.sql.gz?raw=true',
        tag: '{version}-latest-alpine',
        repo: 'dhis2/core',
    }
}

This works, too:

d2 cluster up adev --repo amcgee/dhis2-core --tag dev-alpine --seed --ver 2.32

Spins up amcgee/dhis2-core:dev-alpine with a 2.32 database and deploys it on /adev

if --ver is not passed, it falls back to try to use the name ("adev") as the version. This works if it's dev, 2.32, or something logical. But if a completely custom is used the --ver is recommended.

If the --tag arg is passed, it is used, or it reads the default from the config file and replaces the {version} marker with the resolved version from above.

So d2 cluster up 2.32 --seed and d2 cluster up dev will do the right thing.

varl · 2019-05-16T16:00:18Z

One thing to look out for is that the default config for the entire cli lives in cli-helpers-engine.

varl · 2019-05-16T16:01:36Z

--version is shadowed by the global switch for d2, thinking of renaming --ver to --dhis2-version since that is what we are looking for here.

… decouple-tag-version-name

varl · 2019-05-16T16:14:00Z

This works now. :)

varl · 2019-05-16T16:21:55Z

@amcgee This does not aim to add the API proposed in #45, nor add the single-port gateway, but do the groundwork to decouple some assumptions about Docker repo/tag, DHIS2 versions, context, etc.

Requires this line to be changed: https://github.com/amcgee/dhis2-backend/blob/master/docker-compose.yml#L4 to:

    image: "${DHIS2_CORE_REPO}:${DHIS2_CORE_TAG}"

And either a ~/.config/d2/config.js file needs to exist with the new options, or all args needs to be passed in until the defaults are updated here: https://github.com/dhis2/cli-helpers-engine/blob/master/lib/configDefaults.js#L38-L43 with:

        tag: '{version}-latest-alpine',
        repo: 'dhis2/core',

Would be nice to be able to define the default configuration for the cluster command in the cluster command module, but haven't looked into that yet.

Mohammer5 · 2019-05-17T01:55:05Z

Doesn't really matter, but you could change this:

    const resolvedVersion = ver ? ver : name
    const resolvedTag = tag
        ? tag
        : cluster.tag.replace(/{version}/g, resolvedVersion)
    const resolvedRepo = repo ? repo : cluster.repo
    const resolvedPort = port ? port : cluster.port

to this

    const resolvedVersion = ver || name
    const resolvedTag = tag || cluster.tag.replace(/{version}/g, resolvedVersion)
    const resolvedRepo = repo || cluster.repo
    const resolvedPort = port || cluster.port

Reads a bit nicer, but I don't care, so totally up to you

amcgee · 2019-05-17T11:22:00Z

@varl very nice!

I agree it would be awesome to support per-module config defaults. I'll look into adding that to the engine. In the meantime maybe it's best to just define the defaults in-line here, following on what @Mohammer5 suggested - const resolvedRepo = repo || cluster.repo || defaultRepo would work for that and not require the defaults to be in the engine itself. I think that's a nice first step towards supporting per-cluster defaults.

amcgee · 2019-05-17T11:25:32Z

Is there a reason to configure repo and tag separately? Since we support version-substitution we could just pass an image (or some other name) parameter like dhis2/core:{version}-alpine and not bother with the extra parameters? Just a thought that might simplify this a little.

varl · 2019-05-17T18:59:56Z

Thanks for the input, cleaned it up a bit. 👍

There's more to do but with the defaults strategy we could remove the defaults from the cli-helpers-engine here: https://github.com/dhis2/cli-helpers-engine/blob/master/lib/configDefaults.js#L35-L44

So the responsibility of the cli-helpers-engine would be to load the user's configuration if available, and the default configuration will be handled by the respective command module, demonstrated in defaults.js.

Those defaults can be reused for the default values for e.g. port, so there is a single place to change them if need be.

amcgee

Nice, looks great !! Will test Monday

varl · 2019-05-20T05:31:28Z

One heads up here; this does not support hosting the context from / at the moment, it will always serve DHIS2 from /{name} (which is always the name of the cluster). Personally I think this makes sense.

I can add a --context-path option to the command though where one could override and split the cluster name from the context path. It would introduce another env variable which needs to be used in the docker-compose repo.

const resolvedContext = contextPath || name
...
DHIS2_CORE_CONTEXT_PATH: resolvedContext,

Update

8c4b70a offers this feature.

varl · 2019-05-21T12:53:13Z

8c4b70a adds a --root option to allow for a root context deploy, e.g. http://localhost:8080.

d2 cluster up dev --root

It's possible to set cluster.root to true in the ~/.config/d2/config.js file if that behavior is desirable as default.

varl · 2019-05-21T14:33:11Z

@amcgee I'm pretty much content with this now, ready for review.

varl · 2019-05-21T16:01:41Z

This line: https://github.com/dhis2/cli/pull/46/files#diff-daa1df464973ea1a08c2c122f6a38b65R3

Depends on the outcome of what the image pushed to hub.docker.com/r/dhis2/core is, which will be decided by: https://github.com/dhis2/dhis2-core/pull/3326/files#diff-48b252da11ed2edeae1188be3f75660eR32

Related: dhis2/dhis2-core#3326

varl · 2019-05-21T19:04:04Z

This brings up the question if we should merge amcgee/dhis2-backend to dhis2/docker-compose or something like that, then I can submit a PR for my changes from varl/dhis2-backend.

That is if this PR is the way to move forward for now.

amcgee

I think this looks great and nearly there! I made a couple comments, specifically about defaulting to a root context. The image pattern as-is actually works just fine, but not for released versions. If we expect 2.32-latest in place of 2.32 it would work for both.

packages/cluster/src/commands/up.js

packages/cluster/src/defaults.js

varl · 2019-05-22T12:09:25Z

This is now compatible with master of d2-cluster.

amcgee · 2019-05-23T10:10:49Z

Found an edge case we might want to deal with or call a "known issue":

> d2 cluster up testing --dhis2-version 2.32 --seed
> d2 cluster down testing
> d2 cluster up testing

The last up doesn't have the context to know that it should pass a 2.32 image name, so it will break because amcgee/dhis2-core:testing-alpine doesn't exist. By contrast, when the name is used as the version this works.

To solve this the right way we'll need to keep state about all named containers and their parameters. We probably want to do that anyway, but I'm OK with calling that a future enhancement.

amcgee

Tested and this seems to work well - if we're OK having the above known issue I'm happy with this change! @varl recommend a squash-merge ;-)

varl · 2019-05-23T10:29:20Z

To solve this the right way we'll need to keep state about all named containers and their parameters. We probably want to do that anyway, but I'm OK with calling that a future enhancement.

Ah, shoot. I forgot about this. I ran into the same problem, put it on the back burner and subsequently forgot about it.

I was thinking about wanting to be able to e.g. configure my clusters in the config file:

{
  cluster: {
    instances: {
      '{name}': {
        customContext: true,
        dhis2Version: '2.32',
      }
    }
  }
}

And that it should interact with the cache for un-configured values, which were passed at the time of the first up command.

varl · 2019-05-23T10:30:16Z

if we're OK having the above known issue I'm happy with this change!

Yeah, I think so.

# [1.1.0](v1.0.5...v1.1.0) (2019-05-23) ### Features * decouple tag, name, and version ([#46](#46)) ([28e88e4](28e88e4))

dhis2-bot · 2019-05-23T10:38:55Z

🎉 This PR is included in version 1.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

varl and others added 3 commits May 15, 2019 11:10

feat: decouple tag from version

a7cc765

feat: decouple name from version

1e86159

feat: split image/tag

7d7c66b

varl force-pushed the decouple-tag-version-name branch from 9ff5d06 to 7d7c66b Compare May 15, 2019 09:10

fix: avoid using leading 0 in integers

71ea399

fix: clean up defaults a bit

780c50e

varl marked this pull request as ready for review May 16, 2019 16:04

varl added 3 commits May 16, 2019 18:05

Merge branch 'master' into decouple-tag-version-name

6a5bb88

fix: use dhis2-version instead of ver

842f9d4

Merge branch 'decouple-tag-version-name' of github.com:dhis2/cli into…

9f75c17

… decouple-tag-version-name

varl added 2 commits May 17, 2019 20:54

fix: add defaults file for cluster command module

199be96

fix: make commands use the defaults

01b9458

amcgee reviewed May 17, 2019

View reviewed changes

varl added 2 commits May 18, 2019 09:59

fix: fix args

0587dbd

fix: replace version with resolved

c321b14

varl mentioned this pull request May 18, 2019

fix: move the command configuration to the module itself dhis2/cli-helpers-engine#14

Merged

fix: allow root deploy to instance

8c4b70a

varl added 2 commits May 21, 2019 16:15

fix: pass through defaults

0507883

fix: use defaults

9188443

fix: split context path from name

a0f4124

amcgee requested changes May 22, 2019

View reviewed changes

packages/cluster/src/commands/up.js Outdated Show resolved Hide resolved

packages/cluster/src/defaults.js Outdated Show resolved Hide resolved

packages/cluster/src/defaults.js Outdated Show resolved Hide resolved

varl mentioned this pull request May 22, 2019

feat: allow for custom context deployments of dhis2 core dhis2/d2-cluster-docker-compose#3

Merged

fix: use custom context arg and default to amcgee

a96be13

varl requested a review from amcgee May 22, 2019 12:08

fix: bump the cache name to v2 to transparently bust cache

faa6db5

amcgee approved these changes May 23, 2019

View reviewed changes

varl merged commit 28e88e4 into master May 23, 2019

varl deleted the decouple-tag-version-name branch May 23, 2019 10:36

dhis2-bot pushed a commit that referenced this pull request May 23, 2019

chore(release): cut 1.1.0 [skip ci]

187bd0e

# [1.1.0](v1.0.5...v1.1.0) (2019-05-23) ### Features * decouple tag, name, and version ([#46](#46)) ([28e88e4](28e88e4))

dhis2-bot added the released label May 23, 2019

feat: decouple tag, name, and version #46

feat: decouple tag, name, and version #46

Uh oh!

Conversation

varl commented May 14, 2019

Uh oh!

varl commented May 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

varl commented May 15, 2019

Uh oh!

varl commented May 16, 2019

Uh oh!

varl commented May 16, 2019

Uh oh!

varl commented May 16, 2019

Uh oh!

varl commented May 16, 2019

Uh oh!

varl commented May 16, 2019

Uh oh!

Mohammer5 commented May 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amcgee commented May 17, 2019

Uh oh!

amcgee commented May 17, 2019

Uh oh!

varl commented May 17, 2019

Uh oh!

amcgee left a comment

Choose a reason for hiding this comment

Uh oh!

varl commented May 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Update

Uh oh!

varl commented May 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

varl commented May 21, 2019

Uh oh!

varl commented May 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

varl commented May 21, 2019

Uh oh!

amcgee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

varl commented May 22, 2019

Uh oh!

amcgee commented May 23, 2019

Uh oh!

amcgee left a comment

Choose a reason for hiding this comment

Uh oh!

varl commented May 23, 2019

Uh oh!

varl commented May 23, 2019

Uh oh!

dhis2-bot commented May 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

varl commented May 15, 2019 •

edited

Loading

Mohammer5 commented May 17, 2019 •

edited

Loading

varl commented May 20, 2019 •

edited

Loading

varl commented May 21, 2019 •

edited

Loading

varl commented May 21, 2019 •

edited

Loading