Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-07-18
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
16:22:44 efried still don't agree with the other direction.
16:22:58 dansmith that 1.4 should be able to read a 1.5 file?
16:23:07 efried right. If I lay down a 1.5 file, I expect the 1.5-isms to be observed, not ignored.
16:23:21 dansmith so, here's how an upgrade has to go then:
16:23:23 efried noting that the file declares itself as 1.5
16:23:30 dansmith 1. Start with 1.4 code and config everywhere
16:23:39 dansmith 2. Upgrade to 1.5 code over time
16:23:55 dansmith 3. Once all the 1.5 is deployed, then deploy 1.5 config everywhere, revisiting all nodes and restarting them again
16:24:20 sean-k-mooney Ignoring FFU is that not what we are ment to do in general
16:24:27 efried I see your point, you save the last restart if you do the loose way
16:24:33 sean-k-mooney e.g. upgade code then modify config after
16:24:37 dansmith or else you have to have two versions of all your ansible modules, or they have to embed the version awareness into the ansible modules
16:24:47 dansmith so you're generating the right version for the *Exact* code version you're deploying
16:27:06 efried I'm trying to convince myself that the extra restart is worse than my new fields being ignored, and I don't know why until I go figure out the version diff between the code and the config and which fields were added in that span.
16:27:23 dansmith it's not just the extra restart,
16:27:39 dansmith it's also the embedding of this knowledge into the ansible/puppet/whatever
16:27:57 sean-k-mooney we also might need to do reshapes right?
16:28:27 dansmith well, I guess I'd hope we don't, which is another reason for my other comment,
16:28:40 dansmith which is not allowing operators to randomly change/subtract inventory from what the virt driver is providing
16:28:51 sean-k-mooney im just thinking about FFU whould you have to start the compute agent for each version
16:29:00 sean-k-mooney or just do one config update at the end
16:29:17 efried we've removed all possibilities of reshape and affecting existing inventories from this version of the spec
16:29:28 sean-k-mooney e.g keep 1.2 all the way up to 1.5 cond and then update to 1.5 template or do you have to go lockstepp
16:29:51 sean-k-mooney efried: are you still allowing effect exiting RP
16:29:59 efried yes, but not existing *inventory*
16:30:00 sean-k-mooney by modifying tratis or adding new inventories
16:30:04 efried you can't muck with VCPU
16:30:31 efried and by "existing" I mean "inventories the virt driver deals with explicitly".
16:30:42 sean-k-mooney im not conviced we shoudl allow creating inventories or modifying tratis on the RPs created by the virt driver
16:30:45 sean-k-mooney at least in v1
16:30:51 efried wait, what??
16:30:54 efried That's the entire point
16:31:07 sean-k-mooney i was suggesting only allow creating child RP of the compute node
16:31:14 efried No
16:31:15 sean-k-mooney and giving you full contol of the RP
16:31:16 efried no child RPs

Earlier   Later