dimiternhey GUI team ;)09:26
dimiternI'm trying to import a bundle in the GUI and getting "Error generating changeSet09:27
dimiternThere was an error generating the changeSet. See browser console for additional information"09:27
dimiternthere's nothing in the console log of the browser09:27
dimiternfrankban, fabrice, (others eu-based?) ^^09:28
frankbandimitern: weird that there is nothing in the console09:29
frankbandimitern: could you please paste me the bundle?09:29
dimiternfrankban, yeah - it's https://jujucharms.com/mediawiki-scalable/1009:29
dimiternfrankban, 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
frankbandimitern: uhm... if I just click add to demo in that page the uncommitted state is generated correctly09:30
dimiternfrankban, here's my test env:
dimiternfrankban, 23351c36e3b1df5d7dc256673a2203e909:31
frankbandimitern: so I believe that the GUI server does not know about the spaces constraints09:31
dimiternfrankban, ah :/ too bad09:31
dimiternfrankban, can I patch it somehow for the demo?09:32
frankbandimitern: do you need to deploy the GUI as part of the demo or is it already set up?09:32
dimiternfrankban, I need some way to both show the deployment and deploy the bundle, and I didn't want to use juju-deployer09:33
dimiternfrankban, I deployed the gui first --to 0 09:33
frankbandimitern: ok, but the juju-gui charm is deployed pre-demo right? so that you can hack on it before the demo09:33
dimiternfrankban, yes, absolutely09:34
frankbandimitern: ok, let me deploy it in a local env so that we can try this09:35
dimiternfrankban, I even plan to deploy the bundle before the demo and just show the gui and poke with the CLI a bit09:35
dimiternfrankban, local env won't work for the spaces constraints - they're ignored everywhere but on amazon for the moment09:36
dimiternfrankban, and you need to use 1.25 or master09:36
frankbandimitern: I don't want to deploy the bundle myself, I just want to make it load as uncommitted state09:36
dimiternfrankban, ah, got you ok09:36
dimiternfrankban, fwiw spaces constraints have exactly the same format as tags - comma-delimited,using ^ to indicate "not"09:42
frankbandimitern: ok09:52
frankbandimitern: I was able to ave that bundle imported as uncommitted state09:52
frankbandimitern: first: juju ssh juju-gui/0 sudo nano /usr/local/lib/python2.7/dist-packages/jujubundlelib/validation.py09:52
frankbandimitern: use the editor to update _CONSTRAINTS so that it includes a 'spaces', line09:53
frankbandimitern: then restart the gui server: juju ssh juju-gui/0 sudo service guiserver restart09:53
frankbandimitern: at this poijt your bundle should work09:53
frankbanpoint even09:53
dimiternfrankban, awesome! thanks - will try it now09:54
frankbandimitern: cool09:54
dimiternfrankban, it did import fine after this! thanks again :)09:56
frankbandimitern: yes, we should update bundlelib and make a new charm release with that little change. is spaces constraints for 1.26?09:57
dimiternfrankban, it's for 1.25 even09:57
frankbandimitern: ok, I'll add a card, good that we found this hack for next week09:59
dimiternfrankban, yeah, a bit last-minute, but you know how it is with such demos :)09:59
frankbandimitern: heh, all good10:01
dimiternfrankban, hmm.. there might be more needed btw - i'm watching the deployment as it happens and it seems the spaces constraints were not applied10:03
frankbandimitern: uhm... that's the GUI side10:03
dimiternfrankban, could it be because the gui adds machines first and then deploys --to them, rather than using ServiceDeploy API ?10:04
dimiternfrankban, 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
frankbandimitern: let me check the resulting bundle changes10:06
dimiternfrankban, ok10:07
frankbandimitern: ok so the bundlelib does not handle service constraints, but only machine constraints :-/10:11
dimiternfrankban, right, is there another lib to modify then?10:12
frankbandimitern: so the server side fix is easy enough, not sure about the client side JavaScript10:13
dimiternfrankban, just running: juju add-machine --constraints spaces=dmz did the right thing from the CLI10:13
frankbandimitern: in your bundle, can you add spaces to machines rather than services?10:14
dimiternfrankban, yes - I'll try now10:14
dimiternfrankban, 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:24
frankbandimitern: ok trying10:25
frankbandimitern: ok these are the corresponding changes and they look reasonable (spaces is in there): http://paste.ubuntu.com/12623599/10:27
dimiternfrankban, 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 place10:28
frankbandimitern: 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 env10:28
frankbandimitern: service constraints are not currently handled10:29
frankbandimitern: that's a current limitation of the GUI10:29
frankbandimitern: I mean, the changes do not include service constraints, and this needs to be changed10:29
dimiternfrankban, ok, I'm looking at the network log + ws filter10:29
dimiternfrankban, if we can make at least the machine constraints work, it will be great10:30
frankbandimitern: the machine constraints should already work10:30
dimiternfrankban, but not for spaces=..10:31
dimiternfrankban, unless I need to patch some other place?10:31
frankbandimitern: let's see what's sent to the server10:31
dimiternfrankban, I see no WS requests.. let me try destroying and re-deploying the last bundly10:33
frankbandimitern: to check ws requests we can also try the following: juju set juju-gui builtin-server-logging=debug10:35
dimiternfrankban, I've seen the api WS after refreshing the page, committing now and will see what happens10:35
dimiternfrankban, I've added the logging before this10:36
frankbandimitern: juju ssh juju-gui sudo tailf /var/log/upstart/guiserver.log10:37
dimiternfrankban, there's quite a lot, let me paste it10:38
frankbandimitern: note that it will include passwords10:39
dimiternfrankban, yeah, hence - paste.c.c - https://pastebin.canonical.com/140855/10:39
frankbandimitern: that does not seem to include the deployment calls10:40
dimiternfrankban, what should I look for? Type: ChangeSet ?10:42
frankbandimitern: AddMachines?10:42
dimiternfrankban, just a sec10:43
dimiternfrankban, not found - let me check earlier in the log10:43
dimiternfrankban, this is the full log so far - https://pastebin.canonical.com/140857/10:46
dimiternfrankban, no addmachines 10:46
dimiternfrankban, oops sorry10:47
dimiternfrankban, 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 now10:48
frankbandimitern: ok client -> juju: {"Type":"Client","Request":"AddMachines","Params":{"MachineParams":[{"Jobs":["JobHostUnits"],"Series":"trusty","Constraints":{}}]},"RequestId":13}10:48
frankbandimitern: I've found the bug, and that's a GUI bug10:48
frankbandimitern: the constraints are empty there10:48
dimiternfrankban, ah! ok10:48
dimiternfrankban, yeah, sound like a bug10:49
frankbandimitern: can you confirm that no constraints are applied (not just the spaces one)10:49
dimiternfrankban, yes - none of the services deployed by the gui have any output from juju  get-constraints <svc>10:52
frankbandimitern: 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 AddMachines10:52
dimiternfrankban, nailed it then :)10:52
frankbandimitern: yes looking at the code I can confirm that constraints are ignored by the AddMachine client call10:52
frankbandimitern: 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 point10:54
dimiternfrankban, hmm - wait a sec - is the gui using "AddMachines" client API or "AddMachinesV2" ?10:55
frankbandimitern: {10:55
frankban        Type: 'Client',10:55
frankban        Request: 'AddMachines',10:55
frankban        Params: {MachineParams: machineParams}10:55
frankban      }10:55
frankbandimitern: so facade version shoudl be 0, the request is Client.AddMachines10:56
dimiternfrankban, yeah, actually it doesn't matter - both end up calling the same backend method on the apiserver10:56
dimiternfrankban, so AFAICS it constraints are present in machineParams they are honored10:56
frankbandimitern: they are not, but we can try the ultimate hack10:57
dimiternfrankban, :) sure?10:57
frankbandimitern: so... juju set juju-gui juju-gui-debug=true 10:58
frankbandimitern: this is required so the GUI is served from non-minified files that we can edit on the unit10:58
dimiternfrankban, ok, done10:59
frankbandimitern: now edit the /var/lib/juju-gui/juju-gui/build-debug/juju-ui/store/env/go.js file on the GUI unit10:59
dimiternfrankban, I'm there11:00
frankbandimitern: search for "_addMachines:"11:00
frankban(without quotes)11:01
dimiternfrankban, yeah - the method I presume?11:01
dimitern@method _addMachines11:01
frankbandimitern: add "Constraints: param.constraints," after Series in the machineParam definition11:02
dimiternfrankban, done and saved - restart guiserver or just refresh the gui?11:02
frankbandimitern: just refresh the GUI should be sufficient11:02
frankbandimitern: hard refresh is better (shift + referesh)11:03
dimiternfrankban, I even did ctrl+shit+f5 for good measure - now adding a machine with some constraints via the gui11:04
dimiternfrankban, ..aaand it seems it worked! - I can see r3.xlarge spinning up after adding mem=1600011:05
frankbandimitern: now we should try it with a bundle11:05
dimiternfrankban, yep, but first let me destroy all but the gui first..11:06
dimiternfrankban, ok all is nice and clean now - redeploying the modified bundle with machines constraints specified..11:10
dimiternfrankban, 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:13
frankbandimitern: 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 machines11:14
dimiternfrankban, ah, cool!11:15
frankbandimitern: so you should always be able to use 1, 2 etc.11:15
frankbandimitern: at least that's how we intended it in the spec, hope that the GUI reflects that :-)11:15
dimiternfrankban, ok, here goes nothing ... fingers crossed and all - committing .. :)11:18
frankbandimitern: now it's a good moment to tailf guiserver logs11:18
dimiternfrankban, hmm.. ok - will do - it's doing fine for now though..11:19
frankbandimitern: \o/11:20
dimiternfrankban, hmm.. not really - constraints are still ignored AFAICS - checking the logs now11:22
dimiternfrankban, yeah - AddMachines still has empty constraints11:23
frankbandimitern: okso they are passed when ading machines manually, but they are still ignored when deploying an uncommitted state11:24
dimiternfrankban, 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 them11:25
frankbandimitern: investigating11:26
rick_h_frankban: dimitern only half read the traceback, would this work with your branch frankban ?11:26
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
frankbanrick_h_: my branch?11:27
rick_h_frankban: bundle deployment branch11:27
dimiternrick_h_, hey there :) btw this is actually the first time I'm using the gui for more than 2m :)11:29
frankbanrick_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 that11:29
rick_h_dimitern: sorry it's not going magically for you :/11:29
rick_h_frankban: ok figured I'd ask11:29
frankbanrick_h_: but it's worth trying. my impression is that dimitern was trying to use the GUI, otherwise also the deployer would probably work11:29
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 unfortunately11:30
dimiternrick_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 possible11:30
rick_h_frankban: and if it could use the upcoming stuff that makes him smile...mega win11:30
dimiternrick_h_, +1 me too11:30
dimiternfrankban, rick_h_, guys, I need to step out for ~30m, will ping you when i'm back11:31
frankbanrick_h_: +111:32
rick_h_frankban: if the gui can be made to work that's all good as well11:33
frankbanrick_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 them11:35
rick_h_frankban: understand. 11:36
frankbandimitern: oh wait12:14
frankbandimitern, 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:14
dimiternfrankban, rick_h_, I'm back now12:31
dimiternfrankban, so then change another method?12:32
frankbandimitern, rick_h_ so basically this is fixed in pyramid-fork (not ready to use)12:35
frankbandimitern, rick_h_ looks like we need to backport https://github.com/juju/juju-gui/pull/82112:35
urulamafrankban: is this the only branch needed to be backported?12:36
frankbandimitern: 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 prepareConstraints12:36
frankbandimitern: could you please try to copypasta those lines?12:37
dimiternfrankban, I see.. ok I'll try this diff from the PR now12:37
frankbandimitern: the diff will not apply12:37
frankbandimitern: the pyramid-fork branch has a very different file structure12:38
frankbandimitern: copypaste is your only options, and it's just those lines in the PR12:38
frankbandimitern: you only need the changes in go.js, other stuff are tests and makefile12:38
frankbanurulama: yes that's the only brancvh to be backported for now, but then you'd need a gui release12:39
frankbanurulama: let's see if the hack works12:39
dimiternfrankban, yeah, I'll cherry pick it manually and add the lines 12:39
frankbandimitern: cool, let me know how it goes12:40
urulamafrankban: ok, let's see. we have some gui bugs to fix, so it can land alongside those12:40
dimiternfrankban, sure, will be 1/2h perhaps - need to eat something :)12:40
frankbanurulama: 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 constraints12:41
urulamafrankban: did you already add a defect card for this?12:41
frankbanurulama: it's a task in urgent12:42
frankbanurulama: 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 constraints12:44
urulamafrankban: maybe someone can pick it up even during Seattle12:45
frankbanurulama: 1) might include other changes, since we are modifying the protocol, I have cards for all the changes we want to make12:45
urulamafrankban: cool, ty12:45
frankbanurulama: yes, but I'd like to have a pre-impl call with the implementer before12:46
frankbanurulama: 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 consistency12:49
urulamafrankban: how come 3) will always be 1?12:50
frankbanurulama: because each addUnit change only adds a single unit12:51
frankbanurulama: so if you want to add 10 units you get 10 addUnit changes back12:51
urulamafrankban: ah, ok, misread that num_unit as part of bundle yaml ...12:53
urulamafrankban: multitasking ftw :)12:53
urulamafrankban: ok, noted. this can probably wait till end of Seattle12:53
frankbanurulama: I think so12:54
dimiternfrankban, it *almost* worked :)14:07
dimiternfrankban, {"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
frankbandimitern: so now the constraints are passed, all but the spaces one correct?14:08
dimiternfrankban, 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:08
frankbandimitern: I would not expect anything different ;-)14:09
dimiternfrankban, right - could it be because it's not a string but an array?14:09
frankbandimitern: is it?14:09
dimiternfrankban, well, as json it should be serialized the same way as "tags=foo,^bar" (IIRC "tags: ["foo", "^bar"]")14:10
frankbandimitern: how the server expects spaces to be passed?14:10
frankbandimitern: so the comma separated string is not ok?14:10
dimiternfrankban, nope, it should work the same I guess..14:10
dimiternfrankban, 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:11
frankbandimitern: there is14:12
frankbandimitern: so first I think we should add "spaces" to the genericConstraints list in go.js14:12
frankbandimitern: 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 array14:13
dimiternfrankban, trying it now14:14
frankbandimitern: something like http://pastebin.ubuntu.com/12624773/ I guess14:15
dimiternfrankban, \o/ !! it worked14:20
frankbandimitern: wheeee14:20
dimiternfrankban, I think this gives me enough to prepare for the demo - just let's summarize what was needed in order to get there:14:20
frankbandimitern: well, that was not straightforward, apologies for that, really!14:21
dimiternfrankban, no, it's fine, seriously - we should sync-up more often with you guys and use the gui a *lot* more14:21
dimiternfrankban, would it work if I just patch go.js with the last 2 changes ? (i.e. not touch guiserver ?)14:22
frankbandimitern: you still need to allow spaces in validation.py14:22
dimiternfrankban, 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
frankbandimitern: so the bundle should only include machine constraints14:23
dimiternfrankban, exactly14:23
frankbandimitern: service constraints are ignored14:23
dimiternfrankban, yeah14:24
frankbandimitern: 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 constraints14:24
dimiternfrankban, I see, ok14:24
dimiternfrankban, and also enable the juju set juju-gui juju-gui-debug=true14:24
frankbandimitern: then you need to apply the changes in go.js, excluding the one I suggested intiially, just the backport and the two most recent ones14:25
frankbandimitern: yes, otherwise the minified files are server, and your changes do not have effect14:25
frankban(go.js changes)14:25
frankbandimitern: 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 server14:26
dimiternfrankban, sweet - all written down in my notes, will retry the whole thing from scratch tomorrow, just to make sure it's fine14:27
frankbandimitern: since we are not planning to release a GUI before seattle, you could script that as well 1) deploy 2) set 3) scp 4) ssh14:27
frankbandimitern: sure, and you'll find me here tomorrow14:28
dimiternfrankban, sweet! thanks a million for all your help! :)14:29
frankbanmy pleasure14:29

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!