add cores constraint & alias cpu-cores #6221

Merged
merged 1 commit into from Sep 15, 2016

Conversation

Projects
None yet
2 participants
Contributor

natefinch commented Sep 12, 2016

This fixes-1620056 constraints should support cores=X
Added the concept of aliases of constraint names.
Changed default name of CpuCores to cores.
Added alias from cpu-cores to cores.

This change adds a warning message that is displayed if you specify "cpu-cores" on the CLI, because any error messages will reference "cores" (and everything is displayed as cores). Unfortunately, the way our flag parsing works, there's no way to display this message if the flag parsing fails, so I have to take the constraints as a string flag and then parse during run (at which time we have the ctx and thus can output the correct message).

Most of the changes are trivial conversion of "cpu-cores" to "cores" in tests... for many places it wasn't strictly necessary, we could just use the alias, but this way if we eventually remove the alias, we won't have to update those. Plus, some of them were testing the output, which is always "cores" and it was easier to just change every instance than try to pick out the ones that mattered.

Note, this changes the output of machine hardware characteristics as well, so that change could break some people. It does not change the API representation of cpu-cores for hardware characteristics, so we don't break the API (and that doesn't really matter anyway).

(Review request: http://reviews.vapour.ws/r/5656/)

Contributor

natefinch commented Sep 15, 2016

$$merge$$

Contributor

jujubot commented Sep 15, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Sep 15, 2016

Build failed: Generating tarball failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9243

Contributor

natefinch commented Sep 15, 2016

hmm, weird timeout with requesting gopkg.in repos... will retry
$$merge$$

Contributor

jujubot commented Sep 15, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Sep 15, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9244

Contributor

natefinch commented Sep 15, 2016

$$merge$$

Contributor

jujubot commented Sep 15, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

natefinch commented Sep 15, 2016

helps if I don't break it
$$merge$$

make cpu-cores an alias of cores
This change adds the concept of aliases to constraints, and aliases
the existing cpu-cores as an alternate to the now-standard cores.
This change also removes some problematic reflection code that
relied too heavily on internal knowledge of types and implementation
details.

@jujubot jujubot merged commit e4a75cb into juju:master Sep 15, 2016

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