| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-18 | |||
| 15:05:19 | sean-k-mooney | im currently fixing the other issue but i should get a new version up today. | |
| 15:05:33 | sean-k-mooney | ill start on the glance patch once that is done | |
| 15:08:54 | efried | gibi: responded in https://review.opendev.org/#/c/612497/ -- please let's resolve the discussion about $COMPUTE_HOST if you're still around. | |
| 15:12:04 | cdent | heh: "this is obviously by design" | |
| 15:14:10 | dansmith | efried: your comment about $COMPUTE_HOST for ironic makes this basically unusable by any real ironic deployment, AFAICT | |
| 15:15:22 | efried | dansmith: on the contrary, $COMPUTE_HOST is really a convenience to un-awkward the chicken/egg of having to bring up the compute so it creates the provider so you can glean its UUID and create your file and restart compute. | |
| 15:15:43 | efried | for ironic, don't you already know the UUIDs of the nodes before you bring up the compute service? | |
| 15:15:54 | dansmith | efried: except that ironic computes rebalance their nodes in ways you don | |
| 15:15:58 | dansmith | don't control | |
| 15:16:16 | efried | meaning putting them under different computes? | |
| 15:16:18 | dansmith | the ownership of nodes by comptues I mean | |
| 15:16:21 | efried | got it | |
| 15:16:22 | efried | well | |
| 15:16:34 | dansmith | point being, | |
| 15:16:44 | dansmith | the node is the thing with the inventory, be it the one single node for a libvirt system, | |
| 15:16:56 | dansmith | or the ironic nodes managed by a compute *service* | |
| 15:17:02 | dansmith | further, | |
| 15:17:02 | efried | If we want to add a "template multiple providers" feature in the future, that's certainly a possibility. | |
| 15:17:28 | dansmith | on something like xen or vmware where the compute *service* may be running on a different host than the actual thing providing the resource, | |
| 15:17:48 | dansmith | it's the node that has the inventory and they don't even share a name | |
| 15:18:25 | efried | Okay, so you're suggesting a) $COMPUTE_NODE is the more proper term regardless; and b) we should apply whatever changes to all "nodes", which is only >1 for ironic | |
| 15:18:27 | dansmith | if you make it $COMPUTE_NODE, then I think it would be natural to apply the contents below that to any compute node the compute service manages, | |
| 15:18:36 | dansmith | which is basically the templating thing done | |
| 15:18:41 | dansmith | yes | |
| 15:18:52 | efried | okay, I can buy that. | |
| 15:19:22 | dansmith | every time I look at this spec it has been rev'd or rebased, which makes back-and-forth discussion on individual points kinda hard, | |
| 15:19:38 | dansmith | so I'm going to reply to your reply to my comments on three patch revisions ago to capture this | |
| 15:19:45 | efried | dansmith: thanks. | |
| 15:28:17 | stephenfin | dansmith: Turns out adding a new column to an existing table with 'nullable=False, default=0' doesn't do what you'd think it does. We need 'server_default="0"' instead, for some reason | |
| 15:28:19 | stephenfin | The more you know | |
| 15:28:34 | stephenfin | Django's ORM _does_ work as expected | |
| 15:28:51 | efried | I remember something about this from a couple years ago | |
| 15:28:52 | dansmith | stephenfin: yep, knew that | |
| 15:29:00 | dansmith | stephenfin: however, you don't need a default either | |
| 15:29:13 | dansmith | stephenfin: your object should handle all of that | |
| 15:29:23 | stephenfin | Figured you might, but I mentioned it cos that also took me some time to handle that | |
| 15:29:39 | stephenfin | Hmm, without that SQLite complained about adding a non-nullable field without a default value | |
| 15:29:54 | dansmith | stephenfin: you can't make it non-nullable | |
| 15:30:12 | stephenfin | Right, that's what I ended up doing instead | |
| 15:30:34 | dansmith | it needs to be nullable in the model and the schema, but the object can/should be non-nullable and just handle the translation across the upgrade boundary | |
| 15:31:20 | stephenfin | Oh, I assumed the DB model had to match the object | |
| 15:31:29 | stephenfin | I'll tweak that, in that case | |
| 15:32:11 | dansmith | no, | |
| 15:32:32 | dansmith | that's kind of the point of the object is to hide the differences in the model and the upgrade minutia from the above layers | |
| 15:33:01 | dansmith | efried: I just realized I somehow didn't commit a comment on the spec from yesterday, relating to the version thing | |
| 15:33:22 | dansmith | efried: what I was going to say is that I think that the file should be forwards and backwards compatible within a minor version, | |
| 15:33:26 | dansmith | er, within a major | |
| 15:33:46 | dansmith | such that 1.0 should be parseable by a 1.9-knowing bit of code | |
| 15:34:03 | dansmith | and similarly, 1.9 file should work for a 1.0-knowing old piece of code | |
| 15:34:37 | dansmith | the reason the minor is there is just as an early key for newer code to know if it should even look for new things, not to know if it should use the stricter or looser schecma | |
| 15:34:54 | dansmith | and that the schema(s) should have additionalProperties:true in them | |
| 15:35:19 | dansmith | such that we're always purely additive within a major, and the new stuff is ignore-able by old things | |
| 15:35:28 | dansmith | i.e. less pedantic than our external API | |
| 15:36:25 | dansmith | I say this because in reality people will be rolling multiple versions of compute code with a single version of a deployment tool, and having to know the precise minor expected by a particular RPM revision on a single node is going to be super annoying | |
| 15:37:08 | dansmith | config files are supposed to be uuber stable and our handling of them across versions super gracious | |
| 15:50:19 | gibi | efried: I replyied to the $COMPUTE_HOST part of the spec and removed my -1. Besides this COMPUTE_HOSt identification thing I'm OK with the spec | |
| 15:50:44 | gibi | efried: and I have to leave now (was no a meeting since you pinged) | |
| 15:55:33 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Deprecate 'devname' field of '[pci] passthrough_whitelist' https://review.opendev.org/670585 | |
| 15:59:30 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: migrations: libvirt: remove get_machine_ips() https://review.opendev.org/671471 | |
| 16:00:05 | artom | sean-k-mooney, stephenfin ^^ | |
| 16:00:31 | openstackgerrit | Merged openstack/nova master: bindep: Remove dead markers https://review.opendev.org/671452 | |
| 16:06:53 | artom | stephenfin, cheers! | |
| 16:08:01 | efried | dansmith: I don't see how you could expect 1.4-level code to be able to handle 1.5-level config | |
| 16:10:05 | efried | artom: we talking about remove get_machine_ips? | |
| 16:10:14 | artom | efried, we are | |
| 16:10:16 | efried | looking. | |
| 16:10:24 | artom | Thank you, appreciated :) | |
| 16:11:25 | dansmith | efried: because the rule would be additive only | |
| 16:11:38 | dansmith | efried: it works today with our nova.conf, we just have no version so we don't know anything about it | |
| 16:12:11 | efried | dansmith: wait, you mean 1.4 code would be able to load up 1.5 conf, but it would ignore any 1.5-isms? | |
| 16:14:40 | efried | So schema-wise, the only difference is additionalProperties:true everywhere. But usage-wise, the deployer has to make sure their compute version is at 1.5 to use 1.5-isms -- which they had to do anyway, except now they have to do it explicitly instead of the code enforcing it. | |
| 16:15:12 | efried | s/explicitly/preemptively? or something/ | |
| 16:16:03 | dansmith | I'm not sure what you mean about the explicit operator tasks | |
| 16:16:11 | sean-k-mooney | efried: in v1 of this are we going to allow the provider yaml to modify any RP create by the virt dirver. i read the last version fo the spec as no. we would only beable to add new rps as childern of the copute node | |
| 16:16:15 | efried | I get the parallel with nova.conf, where you can put in all the [foo]bar=baz you want and it'll get ignored by the code. But that's an artifact of oslo.config - we couldn't do anything about that if we tried. | |
| 16:16:22 | dansmith | you mean because you made them put the minor in there? | |
| 16:16:44 | sean-k-mooney | efried: well we could | |
| 16:16:58 | sean-k-mooney | we know all the valid filed we coudl check for any that are not registered | |
| 16:17:03 | sean-k-mooney | and raise an error | |
| 16:17:05 | sean-k-mooney | we just dont | |
| 16:17:14 | efried | dansmith: In both cases I'm assuming a file with schema_version:1.5 and some 1.5-specific values therein. | |
| 16:17:15 | dansmith | sean-k-mooney: we're talking about the opposite thing, and he's right we can't check for that | |
| 16:18:01 | sean-k-mooney | we cant check for fileds and section in the config we dont have a config option registered for? | |
| 16:18:12 | efried | in the looser impl (what you're suggesting), if compute is at 1.4, it will successfully load up the config, but only process the >=1.4-isms, ignoring the 1.5-isms. | |
| 16:18:12 | efried | dansmith: in the stricter impl (as proposed), if compute is at 1.4, it will fail. | |
| 16:18:12 | sean-k-mooney | that seams like ti would be pretty trivail to do | |
| 16:18:24 | dansmith | sean-k-mooney: can you hold on a sec, we're not even talking about the actual config file | |
| 16:18:43 | sean-k-mooney | sure sorry to interupt | |
| 16:19:14 | dansmith | efried: the operator should declare which "version of the docs" they're reading when they write the file, nova should not freak out about things it doesn't know about and not expect things to be present if the operator has clearly indicated they wrote the config file four versions ago | |
| 16:19:25 | dansmith | efried: the alternative is just to look at what is present in the file and not worry about the minor, and only use the major to control large changes, which I think we've already discussed | |
| 16:19:52 | sean-k-mooney | like the version in heat templates? | |
| 16:20:16 | dansmith | efried: but yes, all I'm saying is add additionalProperties:true, and just require the 1.5isms to not break the 1.4isms, and also not have compute fail if it expects 1.5 and only 1.4 is provided (which would break our upgrade promise, which has been in place since forever) | |
| 16:20:40 | dansmith | sean-k-mooney: is the heat template version a format version? I thought that was a self-assigned revision so heat can tell when you've modified it | |
| 16:20:48 | sean-k-mooney | yes | |
| 16:20:53 | sean-k-mooney | its the schema version | |
| 16:21:25 | dansmith | okay, then yes probably similar but I dunno what their semantics are | |
| 16:21:40 | sean-k-mooney | https://docs.openstack.org/heat/latest/template_guide/hot_spec.html | |
| 16:22:02 | sean-k-mooney | heat_template_version | |
| 16:22:03 | sean-k-mooney | This key with value 2013-05-23 (or a later date) indicates that the YAML document is a HOT template of the specified version. | |
| 16:22:08 | efried | dansmith: "1.5 schema and compute always accepts 1.4 config" -- absolutely | |