[09:26] <dimitern> hey GUI team ;)
[09:27] <dimitern> I'm trying to import a bundle in the GUI and getting "Error generating changeSet
[09:27] <dimitern> There was an error generating the changeSet. See browser console for additional information"
[09:27] <dimitern> there's nothing in the console log of the browser
[09:28] <dimitern> frankban, fabrice, (others eu-based?) ^^
[09:29] <frankban> dimitern: weird that there is nothing in the console
[09:29] <frankban> dimitern: could you please paste me the bundle?
[09:29] <dimitern> frankban, yeah - it's https://jujucharms.com/mediawiki-scalable/10
[09:30] <dimitern> frankban, I've modified it slightly for a demo - the only changed thing is adding constraints; the YAML is valid (used yamllint.com) - http://paste.ubuntu.com/12623296/
[09:30] <frankban> dimitern: uhm... if I just click add to demo in that page the uncommitted state is generated correctly
[09:31] <dimitern> frankban, here's my test env: https://52.29.12.21/
[09:31] <dimitern> frankban, 23351c36e3b1df5d7dc256673a2203e9
[09:31] <frankban> dimitern: so I believe that the GUI server does not know about the spaces constraints
[09:31] <dimitern> frankban, ah :/ too bad
[09:32] <dimitern> frankban, can I patch it somehow for the demo?
[09:32] <frankban> dimitern: do you need to deploy the GUI as part of the demo or is it already set up?
[09:33] <dimitern> frankban, I need some way to both show the deployment and deploy the bundle, and I didn't want to use juju-deployer
[09:33] <dimitern> frankban, I deployed the gui first --to 0 
[09:33] <frankban> dimitern: ok, but the juju-gui charm is deployed pre-demo right? so that you can hack on it before the demo
[09:34] <dimitern> frankban, yes, absolutely
[09:35] <frankban> dimitern: ok, let me deploy it in a local env so that we can try this
[09:35] <dimitern> frankban, I even plan to deploy the bundle before the demo and just show the gui and poke with the CLI a bit
[09:36] <dimitern> frankban, local env won't work for the spaces constraints - they're ignored everywhere but on amazon for the moment
[09:36] <dimitern> frankban, and you need to use 1.25 or master
[09:36] <frankban> dimitern: I don't want to deploy the bundle myself, I just want to make it load as uncommitted state
[09:36] <dimitern> frankban, ah, got you ok
[09:42] <dimitern> frankban, fwiw spaces constraints have exactly the same format as tags - comma-delimited,using ^ to indicate "not"
[09:52] <frankban> dimitern: ok
[09:52] <frankban> dimitern: I was able to ave that bundle imported as uncommitted state
[09:52] <frankban> dimitern: first: juju ssh juju-gui/0 sudo nano /usr/local/lib/python2.7/dist-packages/jujubundlelib/validation.py
[09:53] <frankban> dimitern: use the editor to update _CONSTRAINTS so that it includes a 'spaces', line
[09:53] <frankban> dimitern: then restart the gui server: juju ssh juju-gui/0 sudo service guiserver restart
[09:53] <frankban> dimitern: at this poijt your bundle should work
[09:53] <frankban> point even
[09:54] <dimitern> frankban, awesome! thanks - will try it now
[09:54] <frankban> dimitern: cool
[09:56] <dimitern> frankban, it did import fine after this! thanks again :)
[09:57] <frankban> dimitern: yes, we should update bundlelib and make a new charm release with that little change. is spaces constraints for 1.26?
[09:57] <dimitern> frankban, it's for 1.25 even
[09:59] <frankban> dimitern: ok, I'll add a card, good that we found this hack for next week
[09:59] <dimitern> frankban, yeah, a bit last-minute, but you know how it is with such demos :)
[10:01] <frankban> dimitern: heh, all good
[10:03] <dimitern> frankban, hmm.. there might be more needed btw - i'm watching the deployment as it happens and it seems the spaces constraints were not applied
[10:03] <frankban> dimitern: uhm... that's the GUI side
[10:04] <dimitern> frankban, could it be because the gui adds machines first and then deploys --to them, rather than using ServiceDeploy API ?
[10:06] <dimitern> frankban, I've exported the current bundle from the GUI and indeed the constraints from the bundle are not there - http://paste.ubuntu.com/12623497/
[10:06] <frankban> dimitern: let me check the resulting bundle changes
[10:07] <dimitern> frankban, ok
[10:11] <frankban> dimitern: ok so the bundlelib does not handle service constraints, but only machine constraints :-/
[10:12] <dimitern> frankban, right, is there another lib to modify then?
[10:13] <frankban> dimitern: so the server side fix is easy enough, not sure about the client side JavaScript
[10:13] <dimitern> frankban, just running: juju add-machine --constraints spaces=dmz did the right thing from the CLI
[10:14] <frankban> dimitern: in your bundle, can you add spaces to machines rather than services?
[10:14] <dimitern> frankban, yes - I'll try now
[10:24] <dimitern> frankban, I tried importing this bundle - it worked, but still the machine didn't end up in the right place (i.e. spaces constraints were ignored) - http://paste.ubuntu.com/12623583/
[10:25] <frankban> dimitern: ok trying
[10:27] <frankban> dimitern: ok these are the corresponding changes and they look reasonable (spaces is in there): http://paste.ubuntu.com/12623599/
[10:28] <dimitern> frankban, yeah - the machine constraints look sane, but the service it seems it's not supported - that's fine, as long as the machines end up in the right place
[10:28] <frankban> dimitern: in the javascript console, network tab, you should be able to see the websocket requests made by the GUI (there is a WS filter), I am bootstrapping another env
[10:29] <frankban> dimitern: service constraints are not currently handled
[10:29] <frankban> dimitern: that's a current limitation of the GUI
[10:29] <frankban> dimitern: I mean, the changes do not include service constraints, and this needs to be changed
[10:29] <dimitern> frankban, ok, I'm looking at the network log + ws filter
[10:30] <dimitern> frankban, if we can make at least the machine constraints work, it will be great
[10:30] <frankban> dimitern: the machine constraints should already work
[10:31] <dimitern> frankban, but not for spaces=..
[10:31] <dimitern> frankban, unless I need to patch some other place?
[10:31] <frankban> dimitern: let's see what's sent to the server
[10:33] <dimitern> frankban, I see no WS requests.. let me try destroying and re-deploying the last bundly
[10:33] <dimitern> bundle
[10:35] <frankban> dimitern: to check ws requests we can also try the following: juju set juju-gui builtin-server-logging=debug
[10:35] <dimitern> frankban, I've seen the api WS after refreshing the page, committing now and will see what happens
[10:36] <dimitern> frankban, I've added the logging before this
[10:37] <frankban> dimitern: juju ssh juju-gui sudo tailf /var/log/upstart/guiserver.log
[10:38] <dimitern> frankban, there's quite a lot, let me paste it
[10:39] <frankban> dimitern: note that it will include passwords
[10:39] <dimitern> frankban, yeah, hence - paste.c.c - https://pastebin.canonical.com/140855/
[10:40] <frankban> dimitern: that does not seem to include the deployment calls
[10:42] <dimitern> frankban, what should I look for? Type: ChangeSet ?
[10:42] <frankban> dimitern: AddMachines?
[10:43] <dimitern> frankban, just a sec
[10:43] <dimitern> frankban, not found - let me check earlier in the log
[10:46] <dimitern> frankban, this is the full log so far - https://pastebin.canonical.com/140857/
[10:46] <dimitern> frankban, no addmachines 
[10:47] <dimitern> frankban, oops sorry
[10:48] <dimitern> frankban, it seems I've hit a js exception which left the gui paused in the debugger, so the change set was not committed, doing it now
[10:48] <frankban> dimitern: ok client -> juju: {"Type":"Client","Request":"AddMachines","Params":{"MachineParams":[{"Jobs":["JobHostUnits"],"Series":"trusty","Constraints":{}}]},"RequestId":13}
[10:48] <frankban> dimitern: I've found the bug, and that's a GUI bug
[10:48] <frankban> dimitern: the constraints are empty there
[10:48] <dimitern> frankban, ah! ok
[10:49] <dimitern> frankban, yeah, sound like a bug
[10:49] <frankban> dimitern: can you confirm that no constraints are applied (not just the spaces one)
[10:52] <dimitern> frankban, yes - none of the services deployed by the gui have any output from juju  get-constraints <svc>
[10:52] <frankban> dimitern: so the GUI server return a change with constraints (after the  little hack). the GUI has those constraints in its internal uncommitted state, but when turning the uncommitted state into actual API calls the GUI does not send the constraints to AddMachines
[10:52] <dimitern> frankban, nailed it then :)
[10:52] <frankban> dimitern: yes looking at the code I can confirm that constraints are ignored by the AddMachine client call
[10:54] <frankban> dimitern: so we have two GUI bugs (service constraints are ignored server side by the bundlelib, machine constraints are ignored client side by the GUI). I am sorry about that, so the deployer (or quickstart) could be an option at this point
[10:55] <dimitern> frankban, hmm - wait a sec - is the gui using "AddMachines" client API or "AddMachinesV2" ?
[10:55] <frankban> dimitern: {
[10:55] <frankban>         Type: 'Client',
[10:55] <frankban>         Request: 'AddMachines',
[10:55] <frankban>         Params: {MachineParams: machineParams}
[10:55] <frankban>       }
[10:56] <frankban> dimitern: so facade version shoudl be 0, the request is Client.AddMachines
[10:56] <dimitern> frankban, yeah, actually it doesn't matter - both end up calling the same backend method on the apiserver
[10:56] <dimitern> frankban, so AFAICS it constraints are present in machineParams they are honored
[10:57] <dimitern> s/it/if/
[10:57] <frankban> dimitern: they are not, but we can try the ultimate hack
[10:57] <dimitern> frankban, :) sure?
[10:58] <frankban> dimitern: so... juju set juju-gui juju-gui-debug=true 
[10:58] <frankban> dimitern: this is required so the GUI is served from non-minified files that we can edit on the unit
[10:59] <dimitern> frankban, ok, done
[10:59] <frankban> dimitern: now edit the /var/lib/juju-gui/juju-gui/build-debug/juju-ui/store/env/go.js file on the GUI unit
[11:00] <dimitern> frankban, I'm there
[11:00] <frankban> dimitern: search for "_addMachines:"
[11:01] <frankban> (without quotes)
[11:01] <dimitern> frankban, yeah - the method I presume?
[11:01] <dimitern> @method _addMachines
[11:02] <frankban> dimitern: add "Constraints: param.constraints," after Series in the machineParam definition
[11:02] <dimitern> frankban, done and saved - restart guiserver or just refresh the gui?
[11:02] <frankban> dimitern: just refresh the GUI should be sufficient
[11:03] <frankban> dimitern: hard refresh is better (shift + referesh)
[11:04] <dimitern> frankban, I even did ctrl+shit+f5 for good measure - now adding a machine with some constraints via the gui
[11:05] <dimitern> frankban, ..aaand it seems it worked! - I can see r3.xlarge spinning up after adding mem=16000
[11:05] <frankban> dimitern: now we should try it with a bundle
[11:06] <dimitern> frankban, yep, but first let me destroy all but the gui first..
[11:10] <dimitern> frankban, ok all is nice and clean now - redeploying the modified bundle with machines constraints specified..
[11:13] <dimitern> frankban, will it screw things up if I use machine ids like "1", "2", in the bundle? I mean - my last machine ID is 14, so should I change the to: - "X" and machines: "X" to use the next ID 15 etc?
[11:14] <frankban> dimitern: the machine ids in the bundle are not related to the identifiers in the actual environment, it's just a way to assign units to machines
[11:15] <dimitern> frankban, ah, cool!
[11:15] <frankban> dimitern: so you should always be able to use 1, 2 etc.
[11:15] <frankban> dimitern: at least that's how we intended it in the spec, hope that the GUI reflects that :-)
[11:18] <dimitern> frankban, ok, here goes nothing ... fingers crossed and all - committing .. :)
[11:18] <frankban> dimitern: now it's a good moment to tailf guiserver logs
[11:19] <dimitern> frankban, hmm.. ok - will do - it's doing fine for now though..
[11:20] <frankban> dimitern: \o/
[11:22] <dimitern> frankban, hmm.. not really - constraints are still ignored AFAICS - checking the logs now
[11:23] <dimitern> frankban, yeah - AddMachines still has empty constraints
[11:24] <frankban> dimitern: okso they are passed when ading machines manually, but they are still ignored when deploying an uncommitted state
[11:25] <dimitern> frankban, right, so can we fix this? sorry I'm taking so much of your time for this BTW, it's really appreciated - even if we can't fix the above, it should still be possible to do the demo by first manually adding machines and then deploying the bundle's units to them
[11:26] <frankban> dimitern: investigating
[11:26] <rick_h_> frankban: dimitern only half read the traceback, would this work with your branch frankban ?
[11:27] <rick_h_> frankban: dimitern 1 to work around the gui issues and two to perhaps follow a shorter path on the future work?
[11:27] <frankban> rick_h_: my branch?
[11:27] <rick_h_> frankban: bundle deployment branch
[11:29] <dimitern> rick_h_, hey there :) btw this is actually the first time I'm using the gui for more than 2m :)
[11:29] <frankban> rick_h_: machine constraints should work, not the services ones because the changes format reflects the one used by the GUI, and I have cards for fixing that
[11:29] <rick_h_> dimitern: sorry it's not going magically for you :/
[11:29] <rick_h_> frankban: ok figured I'd ask
[11:29] <frankban> rick_h_: but it's worth trying. my impression is that dimitern was trying to use the GUI, otherwise also the deployer would probably work
[11:30] <rick_h_> dimitern: this is part of the fun of keeping up with core features when we miss/don't have the support through all the tool chain unfortunately
[11:30] <dimitern> rick_h_, it's totally fine, not complaining :)
[11:30] <rick_h_> frankban: well for a demo in front of sabdfl I'd like to avoid using the deployer if at all possible
[11:30] <rick_h_> frankban: and if it could use the upcoming stuff that makes him smile...mega win
[11:30] <dimitern> rick_h_, +1 me too
[11:31] <dimitern> frankban, rick_h_, guys, I need to step out for ~30m, will ping you when i'm back
[11:32] <frankban> rick_h_: +1
[11:33] <rick_h_> frankban: if the gui can be made to work that's all good as well
[11:35] <frankban> rick_h_: I've found the problem with the API call, and now machine constraints work with the hack I described above if you manually add a new machine. they are still ignored if you use ecs. I checked and ecs.changeSet includes the correct args as returned by the GUI server, so we'll need to investigate where we loose them
[11:36] <rick_h_> frankban: understand. 
[12:14] <frankban> dimitern: oh wait
[12:14] <frankban> dimitern, rick_h_ actually go.js handles machine constraints, I didn't see the if (param.constraints) {
[12:14] <frankban>           machineParam.Constraints = self.prepareConstraints(param.constraints);
[12:14] <frankban>         }
[12:31] <dimitern> frankban, rick_h_, I'm back now
[12:32] <dimitern> frankban, so then change another method?
[12:35] <frankban> dimitern, rick_h_ so basically this is fixed in pyramid-fork (not ready to use)
[12:35] <frankban> dimitern, rick_h_ looks like we need to backport https://github.com/juju/juju-gui/pull/821
[12:36] <urulama> frankban: is this the only branch needed to be backported?
[12:36] <frankban> dimitern: so basically the change that needs to be done in go.js is not the one I suggested, but the one at https://github.com/juju/juju-gui/pull/821/files. so add a bunch of lines at the beginning of prepareConstraints
[12:37] <frankban> dimitern: could you please try to copypasta those lines?
[12:37] <dimitern> frankban, I see.. ok I'll try this diff from the PR now
[12:37] <frankban> dimitern: the diff will not apply
[12:38] <frankban> dimitern: the pyramid-fork branch has a very different file structure
[12:38] <frankban> dimitern: copypaste is your only options, and it's just those lines in the PR
[12:38] <frankban> dimitern: you only need the changes in go.js, other stuff are tests and makefile
[12:39] <frankban> urulama: yes that's the only brancvh to be backported for now, but then you'd need a gui release
[12:39] <frankban> urulama: let's see if the hack works
[12:39] <dimitern> frankban, yeah, I'll cherry pick it manually and add the lines 
[12:40] <frankban> dimitern: cool, let me know how it goes
[12:40] <urulama> frankban: ok, let's see. we have some gui bugs to fix, so it can land alongside those
[12:40] <dimitern> frankban, sure, will be 1/2h perhaps - need to eat something :)
[12:41] <frankban> urulama: this is actually a big bug, meaning machine constraints are not applied when deploying bundles. it didn't hit us because there are not so many bundles with constraints
[12:41] <urulama> frankban: did you already add a defect card for this?
[12:42] <frankban> urulama: it's a task in urgent
[12:42] <urulama> kk
[12:44] <frankban> urulama: what we need after seattle is 1) update jujubundlelib so that it sends service constraints and it allows "spaces" as a constraint 2) update the GUI ecs to reflect the changes in jujubundlelib, 3) release jujubundlelib and the GUI 4) update bundlechanges as well 5) update guibundles to use newer bundlechanges and to send the service constraints
[12:45] <urulama> frankban: maybe someone can pick it up even during Seattle
[12:45] <frankban> urulama: 1) might include other changes, since we are modifying the protocol, I have cards for all the changes we want to make
[12:45] <urulama> frankban: cool, ty
[12:46] <frankban> urulama: yes, but I'd like to have a pre-impl call with the implementer before
[12:49] <frankban> urulama: these are the changes I'd like to make to jujubundlelib/bundlechanges protocol: 1) support service constraints, 2) in addService changes, make the charm argument a placeholder pointing to the correspondig addCharm change, 3) remove the num_unit argument from addUnit changes, since it will always be 1 unit, 4) maybe s/deploy/addService in the addService method, for consistency
[12:50] <urulama> frankban: how come 3) will always be 1?
[12:51] <frankban> urulama: because each addUnit change only adds a single unit
[12:51] <frankban> urulama: so if you want to add 10 units you get 10 addUnit changes back
[12:53] <urulama> frankban: ah, ok, misread that num_unit as part of bundle yaml ...
[12:53] <urulama> frankban: multitasking ftw :)
[12:53] <urulama> frankban: ok, noted. this can probably wait till end of Seattle
[12:54] <frankban> urulama: I think so
[14:07] <dimitern> frankban, it *almost* worked :)
[14:08] <dimitern> frankban, {"Type":"Client","Request":"AddMachines","Params":{"MachineParams":[{"Jobs":["JobHostUnits"],"Series":"trusty","Constraints":{"arch":"amd64","cpu-cores":1,"cpu-power":300,"mem":3840,"root-disk":8192}}]},"RequestId":21}
[14:08] <frankban> dimitern: so now the constraints are passed, all but the spaces one correct?
[14:08] <dimitern> frankban, spaces are not included though - the bundle has this line: constraints: "arch=amd64 cpu-cores=1 cpu-power=300 mem=3840 root-disk=8192 spaces=dmz"
[14:09] <frankban> dimitern: I would not expect anything different ;-)
[14:09] <dimitern> frankban, right - could it be because it's not a string but an array?
[14:09] <frankban> dimitern: is it?
[14:10] <dimitern> frankban, well, as json it should be serialized the same way as "tags=foo,^bar" (IIRC "tags: ["foo", "^bar"]")
[14:10] <frankban> dimitern: how the server expects spaces to be passed?
[14:10] <frankban> dimitern: so the comma separated string is not ok?
[14:10] <dimitern> frankban, nope, it should work the same I guess..
[14:11] <dimitern> frankban, I can't see any specific handling for "tags" or "networks" (I wouldn't expect it for the latter, but at least the former is useful)
[14:12] <frankban> dimitern: there is
[14:12] <frankban> dimitern: so first I think we should add "spaces" to the genericConstraints list in go.js
[14:13] <frankban> dimitern: the in prepareConstraints there is a piece of code like http://pastebin.ubuntu.com/12624766/ and we should do the same for spaces if we need to send an array
[14:14] <dimitern> frankban, trying it now
[14:15] <frankban> dimitern: something like http://pastebin.ubuntu.com/12624773/ I guess
[14:20] <dimitern> frankban, \o/ !! it worked
[14:20] <frankban> dimitern: wheeee
[14:20] <dimitern> frankban, I think this gives me enough to prepare for the demo - just let's summarize what was needed in order to get there:
[14:21] <frankban> dimitern: well, that was not straightforward, apologies for that, really!
[14:21] <dimitern> frankban, no, it's fine, seriously - we should sync-up more often with you guys and use the gui a *lot* more
[14:22] <dimitern> frankban, would it work if I just patch go.js with the last 2 changes ? (i.e. not touch guiserver ?)
[14:22] <frankban> dimitern: you still need to allow spaces in validation.py
[14:23] <dimitern> frankban, I guess since service cons are not used, it's not worth specifying them in the bundle for services, just machines?
[14:23] <dimitern> (I have them in both places now)
[14:23] <frankban> dimitern: so the bundle should only include machine constraints
[14:23] <dimitern> frankban, exactly
[14:23] <frankban> dimitern: service constraints are ignored
[14:24] <dimitern> frankban, yeah
[14:24] <frankban> dimitern: still you'll need to add 'spaces' to _CONSTRAINTS in validation.py, because I believe the same function is used to validate service and machine constraints
[14:24] <dimitern> frankban, I see, ok
[14:24] <dimitern> frankban, and also enable the juju set juju-gui juju-gui-debug=true
[14:25] <frankban> dimitern: then you need to apply the changes in go.js, excluding the one I suggested intiially, just the backport and the two most recent ones
[14:25] <frankban> dimitern: yes, otherwise the minified files are server, and your changes do not have effect
[14:25] <frankban> (go.js changes)
[14:26] <frankban> dimitern: so, for sanity, I'd suggest to 1) deploy the GUI 2) apply the juju-gui-debug=true option 3) change validation.py and go.js and 4) restart the GUI server
[14:27] <dimitern> frankban, sweet - all written down in my notes, will retry the whole thing from scratch tomorrow, just to make sure it's fine
[14:27] <frankban> dimitern: since we are not planning to release a GUI before seattle, you could script that as well 1) deploy 2) set 3) scp 4) ssh
[14:28] <frankban> dimitern: sure, and you'll find me here tomorrow
[14:29] <dimitern> frankban, sweet! thanks a million for all your help! :)
[14:29] <frankban> my pleasure