/juju

put add-/list-image behind a feature flag #4093

Merged
merged 1 commit into from Jan 13, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@wallyworld @jujubot
@wallyworld
Owner

wallyworld commented Jan 13, 2016

Do not expose add/list-image by default - use a "image-metadata" feature flag.

(Review request: http://reviews.vapour.ws/r/3514/)
@wallyworld
Owner

wallyworld commented Jan 13, 2016

$$merge$$
@jujubot
Contributor

jujubot commented Jan 13, 2016

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

jujubot commented Jan 13, 2016

Build failed: Does not match ['fixes-1532777']
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/5918
@wallyworld
Owner

wallyworld commented Jan 13, 2016

$$JFDI$$
@jujubot
Contributor

jujubot commented Jan 13, 2016

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

jujubot added a commit that referenced this pull request Jan 13, 2016

@jujubot
Merge pull request #4093 from wallyworld/add-image-ff 
put add-/list-image behind a feature flag

Do not expose add/list-image by default - use a "image-metadata" feature flag.

(Review request: http://reviews.vapour.ws/r/3514/)
69899cc

@jujubot jujubot merged commit 69899cc into juju:master Jan 13, 2016

axw added a commit to axw/juju that referenced this pull request Jan 13, 2016

@axw
put --bootstrap-iamge behind feature flag 
Per #4093, image metadata related bits are
going behind a feature flag for now.
3687162

@axw axw referenced this pull request Jan 13, 2016

Merged

put --bootstrap-iamge behind feature flag #4094

jujubot added a commit that referenced this pull request Jan 13, 2016

@jujubot
Merge pull request #4094 from axw/bootstrap-image-feature-flag 
put --bootstrap-iamge behind feature flag

Per #4093, image metadata related bits are
going behind a feature flag for now.

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

cherylj pushed a commit to cherylj/juju that referenced this pull request Jan 14, 2016

Revert "Merge pull request #4093 from wallyworld/add-image-ff" 
This reverts commit 69899cc, reversing
changes made to 26de1ef.
7f05cd6

jujubot added a commit that referenced this pull request Jan 14, 2016

@jujubot
Merge pull request #4117 from cherylj/fix_CR_windows_test 
Revert injecting PR #4093

Reverted the addition of the feature flag setting in the plugin, but left the feature flag as a later commit depended on it.

(Review request: http://reviews.vapour.ws/r/3539/)
33f27d3

cherylj pushed a commit to cherylj/juju that referenced this pull request Jan 17, 2016

1534201: Re-enable feature flag for juju-metadata 
In this patch, I cherry-picked the original commit:
juju#4093

And changed the following:

1 - Separated out main() into a windows and nix
file, and used the same method as jujud for
setting feature flags.

2 - In tests, updated the setting of ps.Env to not
include the feature flag.  The way it was before was
incorrect and just appended "image-metadata" to the
Env which confused windows and led to the command not
being run at all.  It is not needed for ubuntu tests.

3 - Added an assert to getHelpCommandNames to verify
the length of the SplitAfter result before reading
index 1 (this was causing a panic due to no output
from point 2 above).

4 - Skipped tests which verify feature-flag enabled
behavior.  Bug 1534307 was opened to add it back in
at a later time.

5 - assertHelpOutput now does a regex match for the
command name as in windows, as a warning will appear
in the output about not being able to get the
registry key (it is not set as part of testing, see
bug 1493887).

I have manually verified that add-image and list-images
only show up in ubuntu when using the feature flag.

For windows, I was unable to set the feature flags,
but did verify that add-image and list-images were
not enabled by default.

Unit tests pass on my ubuntu system and on the
windows system provided by sinzui.
d8b4b83

@cherylj cherylj referenced this pull request Jan 17, 2016

Merged

1534201: Re-enable feature flag for juju-metadata #4130

jujubot added a commit that referenced this pull request Jan 18, 2016

@jujubot
Merge pull request #4130 from cherylj/1534201 
1534201: Re-enable feature flag for juju-metadata

In this patch, I cherry-picked the original commit:
#4093
  
And changed the following:

1 - Separated out main() into a windows and nix
file, and used the same method as jujud for
setting feature flags.

2 - In tests, updated the setting of ps.Env to not
include the feature flag.  The way it was before was
incorrect and just appended "image-metadata" to the
Env which confused windows and led to the command not
being run at all.  It is not needed for ubuntu tests.
  
3 - Added an assert to getHelpCommandNames to verify
the length of the SplitAfter result before reading
index 1 (this was causing a panic due to no output
from point 2 above).
    
4 - Skipped tests which verify feature-flag enabled
behavior.  Bug 1534307 was opened to add it back in
at a later time.
  
5 - assertHelpOutput now does a regex match for the
command name as in windows, as a warning will appear
in the output about not being able to get the
registry key (it is not set as part of testing, see
bug 1493887).
    
I have manually verified that add-image and list-images
only show up in ubuntu when using the feature flag.
    
For windows, I was unable to set the feature flags,
but did verify that add-image and list-images were
not enabled by default.
   
Unit tests pass on my ubuntu system and on the
windows system provided by sinzui.


(Review request: http://reviews.vapour.ws/r/3552/)
8e974a6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment