Open Stack

On Mon, 2018-01-15 at 12:30 -0500, Jay Pipes wrote:
> On 01/15/2018 11:45 AM, mdulko at redhat.com wrote:
> > Hi,
> > 
> > os-vif commit [1] introduced a non-backward compatible change to the
> > Subnet object - removal of ips field. Turns out kuryr-kubernetes were
> > depending on that e.g. here [1] and we're now broken with os-vif 1.8.0.
> > 
> > kuryr-kubernetes is saving the VIF objects into the K8s resources
> > annotations, so to keep backwards compatibility we need
> > VIFBase.obj_make_compatible able to backport the data back into the
> > Subnet object. Or be able to load the older data to the newer object.
> > Anyone have an advice how we should proceed with that issue?
> 
> It would have been great to know kuryr-kubernetes was saving/using these 
> objects :) as mentioned on the original os-vif code review, the 
> versioned objects in os-vif have yet to be used in over-the-wire 
> communication nor have they been saved to a backing data store by Nova 
> or Neutron. Thus, we haven't bothered with the obj_make_compatible() 
> stuff yet.
> 
> If we had known there was a non-Nova non-Neutron client of os-vif, of 
> course we would have been tracking changes using obj_make_compatible().

Sure thing, I've already learned that without CI breaking things is
expected. :)

> That said, even if we *were* using obj_make_compatible() and allowing 
> for the backversioning of object formats, that would not have magically 
> enabled kuryr-kubernetes to work with our objects without modification. 
> kuryr-kubernetes would still need to do the dance of telling os-vif 
> somehow what version of the objects that it expects to be given. This is 
> what all the infrastructure in oslo.versionedobject's client-server 
> negotiation does and it's non-trivial.

Agreed, though now the only solution is either to implement backporting
on kuryr-kubernetes side or drop backward compatibility with already
existing deployments.

> Bottom line, we can straight revert the os-vif patch in question 
> (because it's really just a cleanup), release os-vif 1.8.1 by the cutoff 
> on Thursday and "fix" kuryr-kubernetes. However, we will want to have a 
> call with you guys to tell you exactly how to do the versioning 
> negotiation that you will now need to do since you're storing these 
> objects somewhere.

If that change isn't critical to anyone, I'd prefer to do the revert
now and discuss how to do such changes correctly in the future. 

Implementing obj_make_compatible would help us, but I'm not sure it's a
good long-term solution, because it would require kuryr-kubernetes to
*always* specify the version of all the o.vo it uses, which may lead to
locking it to the lowest version that's available. In case of o.vo used
for DB & RPC communication that's solved by online data migrations. We
don't have such framework now and I simply don't know if we should copy
the same approach.

> Best,
> -jay
> 
> > It would also be nice to setup a kuryr-kubernetes gate on the os-vif
> > repo. If there are no objections to that I'd volunteer to submit a
> > commit that adds it.
> > 
> > Thanks,
> > Michal
> > 
> > [1] https://review.openstack.org/#/c/508498
> > [2] https://github.com/openstack/kuryr-kubernetes/blob/18db6499432e6cab61059eb5abeeaad3ea40b6e4/kuryr_kubernetes/cni/binding/base.py#L64-L66
> > 
> > __________________________________________________________________________
> > OpenStack Development Mailing List (not for usage questions)
> > Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> > 
> 
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev