Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Add support for passing a charm/bundle id as a deploy-target query parameter #624

Merged
merged 5 commits into from
Oct 23, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,8 @@ YUI.add('juju-gui', function(Y) {
this._renderHelpDropdownView();
this._renderDeployerBarView();
this._renderEnvironmentHeaderView();
this.get('subApps').charmbrowser.on(
'*:autoplaceAndCommitAll', this._autoplaceAndCommitAll, this);
}, this);

this.zoomMessageHandler = Y.one(Y.config.win).on('resize', function(e) {
Expand Down Expand Up @@ -859,6 +861,18 @@ YUI.add('juju-gui', function(Y) {
this.deployerBar.addTarget(this);
},

/**
When the user provides a charm id in the deploy-target query param we want
to auto deploy that charm. This calls the appropriate methods in the
deployer bar to auto place any outstanding units and deploy the charm.

@method _autoplaceAndCommitAll
*/
_autoplaceAndCommitAll: function() {
this.deployerBar._autoPlaceUnits();
this.deployerBar.deploy();
Copy link
Contributor

Choose a reason for hiding this comment

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

can we not just 'deploy' with immediate true? I guess that doesn't help with placement first, just ecs or not to ecs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We still won't have a placed unit but that might be a better approach even if just for the deploy command. I'll look into that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After looking into this, using immediate: true will help deploy the charm faster but we will still need to create a machine and place the unit on it so overall it won't gain us anything. Going to leave it as-is

},

/**
* Toggle the visibility of the sidebar.
*
Expand Down
48 changes: 48 additions & 0 deletions app/subapps/browser/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ YUI.add('subapp-browser', function(Y) {
// model to be inspected will not be available.
allowInspector: !cfg.sandbox,
dispatchers: {
app: {
deployTarget: this._deployTargetDispatcher.bind(this)
},
sectionA: {
charmbrowser: this._charmBrowserDispatcher.bind(this),
inspector: this._inspectorDispatcher.bind(this),
Expand Down Expand Up @@ -316,6 +319,51 @@ YUI.add('subapp-browser', function(Y) {
}, this);
},

/**
Handles deploying the charm or bundle id passed in as a deploy-target
as a query param on app load.

Be sure to use the fully qualified urls like cs:precise/mysql-48 and
bundle:mediawiki/7/single because it make it easy to check what type of
entity the id is referring to.

@method _deployTargetDispatcher
@param {String} entityId The id of the charm or bundle to deploy.
*/
_deployTargetDispatcher: function(entityId) {
var store = this.get('store');
if (entityId.indexOf('bundle') === 0) {
// Query the charmstore for the bundle data
Copy link
Contributor

Choose a reason for hiding this comment

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

full sentence please

store.bundle(entityId.replace(':', '/'), {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we replace just : here and cs: down below in 344?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The api only accepts requests in the format bundle/foo/5/bar for bundles and precise/foo for the charms. This seems like an inconsistency in the API but maybe that's fixed in v4?

Copy link
Contributor

Choose a reason for hiding this comment

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

yea, v4 will be much nicer on that front.

'success': function(bundle) {
this.get('deployBundle')(bundle.data, bundle.id);
}
}, this);
} else {
// If it's not a bundle then it's a charm.
store.charm(entityId.replace('cs:', ''), {
'success': function(data) {
var charm = data.charm,
config = {};
Object.keys(charm.options).forEach(function(key) {
config[key] = charm.options[key]['default'];
});
// We call the env deploy method directly because we don't want the
// ghost inspector to open.
this.get('env').deploy(
charm.id,
charm.name,
config,
undefined, //config file content
1, // number of units
{}, //constraints
null); // toMachine
this.fire('autoplaceAndCommitAll');
}.bind(this)
});
}
},

/**
Handles rendering and/or updating the charmbrowser UI component.

Expand Down
6 changes: 6 additions & 0 deletions app/utils/environment-change-set.js
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,12 @@ YUI.add('environment-change-set', function(Y) {
@param {Object} db The database instance.
*/
prepare: function(db) {
if (!this.options || !this.options.modelId) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change and the following similar change are tested as a side effect of existing tests. The app falls over without these modifications.

// When using the deploy-target query parameter we want to auto
// deploy so we can skip generating the line item in the deployer
// bar.
return;
}
var ghostService = db.services.getById(this.options.modelId);
// Update the service name, which can change from when the
// charm is added to the canvas to the actual time the changes are
Expand Down
45 changes: 41 additions & 4 deletions app/views/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ YUI.add('juju-app-state', function(Y) {
@param {Object} state The current state object.
*/
dispatch: function(state) {
var sections = ['sectionA', 'sectionB'];
var sections = ['app', 'sectionA', 'sectionB'];
Copy link
Contributor

Choose a reason for hiding this comment

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

oh wow, a new section eh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this way it keeps the search and all of the other sections working as expected. This functionality also doesn't really fit in any of the sections so placing it in a 'parent' one made sense.

// If the component of a section has changed then clean out that section.
sections.forEach(function(section) {
if (this.hasChanged(section, 'component')) {
Expand All @@ -135,13 +135,29 @@ YUI.add('juju-app-state', function(Y) {
*/
_dispatchSection: function(section, state) {
var sections = {
app: 'App',
sectionA: 'SectionA',
sectionB: 'SectionB'
};
// calls _dispatchSectionA or _dispatchSectionB
this['_dispatch' + sections[section]](state);
},

/**
Calls the dispatcher subscribed on instantiation for the overall app.

@method _dispatchApp
@param {Object} state App's state object.
*/
_dispatchApp: function(state) {
var dispatchers = this.get('dispatchers');
if (Y.Lang.isObject(state)) {
Object.keys(state).forEach(function(key) {
dispatchers.app[key](state[key]);
});
}
},

/**
Calls the dispatcher subscribed on instantiation for the sectionA
component.
Expand Down Expand Up @@ -399,12 +415,16 @@ YUI.add('juju-app-state', function(Y) {
});
}
}, this);
// There's always a query component, but we only care if it reflects a
// search.
// There's always a query component, if it reflects a search.
if (query && (query.text || query.categories)) {
// `state` is passed in by reference and modified in place.
this._addQueryState(state, query);
}
// For demonstration purposes it's nice to open the GUI to an already
// deployed bundle or charm.
if (query && query['deploy-target']) {
this._addDeployTarget(state, query);
}
return state;
},
/**
Expand All @@ -416,14 +436,31 @@ YUI.add('juju-app-state', function(Y) {

@method _addQueryState
@param {Object} state The built state object.
@param {String} query The text of the search query value.
@param {Object} query The query param object.
*/
_addQueryState: function(state, query) {
Y.namespace.call(state, 'sectionA.metadata.search');
// This gets passed the entire query even fields which are not search
// related.
state.sectionA.metadata.search = query;
this.filter.update(query);
},

/**
Add the deploy target into the state object. Unlike the _addQueryState
method which adds the entire query into the search metadata this only
adds the deploy-target component to the deployTarget component in the
state object.

@method _addDeployTarget
@param {Object} state The built state object.
@param {Object} query The query param object.
*/
_addDeployTarget: function(state, query) {
Y.namespace.call(state, 'app.deployTarget');
state.app.deployTarget = query['deploy-target'];
},

/**
Adds the supplied configuration parameters into the section defined.

Expand Down
10 changes: 9 additions & 1 deletion app/widgets/deployer-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ YUI.add('deployer-bar', function(Y) {
@param {Object} evt The event object.
*/
deploy: function(evt) {
evt.halt();
if (evt && typeof evt.halt === 'function') {
evt.halt();
}
var container = this.get('container'),
ecs = this.get('ecs'),
autodeploy = container.one('input[value="autodeploy"]');
Expand Down Expand Up @@ -513,6 +515,12 @@ YUI.add('deployer-bar', function(Y) {
// units as follow up.
switch (change.command.method) {
case '_deploy':
if (!change.command.options || !change.command.options.modelId) {
// When using the deploy-target query parameter we want to auto
// deploy so we can skip generating the line item in the deployer
// bar.
return;
}
var ghostService = db.services.getById(
change.command.options.modelId);
changeItem.icon = 'changes-service-added';
Expand Down
29 changes: 27 additions & 2 deletions test/test_app.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ function injectData(app, data) {
Y.juju.App.prototype, '_renderDeployerBarView');
context._cleanups.push(_renderDeployerBarView.reset);
}
app = new Y.juju.App(config);
app = new Y.juju.App(Y.mix(config, {consoleEnabled: true}));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Driveby fix to enable console logging in the app tests

app.navigate = function() {};
app.showView(new Y.View());
injectData(app);
Expand Down Expand Up @@ -280,6 +280,30 @@ function injectData(app, data) {
});
});

it('attaches a handler for autoplaceAndCommitAll event', function(done) {
constructAppInstance({}, this);
app._autoplaceAndCommitAll = function() {
// This test will hang if this method is not called from the following
// event being fired.
done();
};
app.after('ready', function() {
app.get('subApps').charmbrowser.fire('autoplaceAndCommitAll');
});
});

it('autoplaceAndCommitAll places and deploys', function() {
constructAppInstance({}, this);
app.deployerBar = {
_autoPlaceUnits: utils.makeStubFunction(),
deploy: utils.makeStubFunction(),
destroy: function() {}
};
app._autoplaceAndCommitAll();
assert.equal(app.deployerBar._autoPlaceUnits.callCount(), 1);
assert.equal(app.deployerBar.deploy.callCount(), 1);
});

it('should display a zoom message on small browsers', function() {
constructAppInstance({
env: new juju.environments.GoEnvironment({
Expand Down Expand Up @@ -465,7 +489,8 @@ describe('File drag over notification system', function() {
conn: {
send: function() {},
close: function() {}
}
},
ecs: new juju.EnvironmentChangeSet()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Driveby for another spurious test failure fix

});
}
if (config.env && config.env.connect) {
Expand Down
59 changes: 59 additions & 0 deletions test/test_browser_app.js
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,65 @@ with this program. If not, see <http://www.gnu.org/licenses/>.
});
});

describe('_deployTargetDispatcher', function() {
it('requests bundle id from store then deploys', function() {
app.set('store', {
bundle: utils.makeStubFunction()
});
app.set('deployBundle', utils.makeStubFunction());
app._deployTargetDispatcher('bundle:elasticsearch/15/cluster');
var store = app.get('store');
assert.equal(store.bundle.callCount(), 1);
var bundleArgs = store.bundle.lastArguments();
assert.equal(bundleArgs[0], 'bundle/elasticsearch/15/cluster',
'api requires ids to have \/ instead of the : from the id');
// The second param is the callback for the store response. So
// we need to manually trigger it to test it.
var bundleData = {data: 'foo', id: 'bar'};
bundleArgs[1].success.call(app, bundleData);
var deploy = app.get('deployBundle');
assert.equal(deploy.callCount(), 1);
assert.deepEqual(
deploy.lastArguments(),
[bundleData.data, bundleData.id]);
assert.deepEqual(bundleArgs[2], app);
});

it('requests charm id from store then deploys', function() {
app.setAttrs({
store: { charm: utils.makeStubFunction() },
env: { deploy: utils.makeStubFunction() }
});
app._deployTargetDispatcher('cs:precise/apache2-25');
var store = app.get('store');
assert.equal(store.charm.callCount(), 1);
var charmArgs = store.charm.lastArguments();
assert.equal(charmArgs[0], 'precise/apache2-25',
'api requires removal of cs:');
// The second param is the callback for the store response. So we
// need to manually trigger it to test it.
var charmData = {
charm: {
id: 'foo',
name: 'bar',
options: {
opt1: { 'default': 'opt1default' }
}
}
};
charmArgs[1].success(charmData);
var deploy = app.get('env').deploy;
assert.equal(deploy.callCount(), 1);
var deployArgs = deploy.lastArguments();
assert.equal(deployArgs[0], charmData.charm.id);
assert.equal(deployArgs[1], charmData.charm.name);
assert.deepEqual(deployArgs[2], { opt1: 'opt1default' });
assert.strictEqual(deployArgs[3], undefined);
assert.equal(deployArgs[4], 1);
assert.deepEqual(deployArgs[5], {});
assert.strictEqual(deployArgs[6], null);
});
});

describe('_machine', function() {
var renderMachineStub, setSelectedStub;
Expand Down