Open Stack

On 10/23/18 9:55 PM, Adrian Turjak wrote:
> 
> On 24/10/18 2:09 AM, Ben Nemec wrote:
>>
>>
>> On 10/22/18 5:40 PM, Matt Riedemann wrote:
>>> On 10/22/2018 4:35 PM, Adrian Turjak wrote:
>>>>> The one other open question I have is about the Adjutant change [2]. I
>>>>> know Adjutant is very new and I'm not sure what upgrades look like for
>>>>> that project, so I don't really know how valuable adding the upgrade
>>>>> check framework is to that project. Is it like Horizon where it's
>>>>> mostly stateless and fed off plugins? Because we don't have an upgrade
>>>>> check CLI for Horizon for that reason.
>>>>>
>>>>> [1]
>>>>> https://review.openstack.org/#/q/topic:upgrade-checkers+(status:open+OR+status:merged)
>>>>>
>>>>> [2]https://review.openstack.org/#/c/611812/
>>>>>
>>>> Adjutant's codebase is also going to be a bit unstable for the next few
>>>> cycles while we refactor some internals (we're not marking it 1.0 yet).
>>>> Once the current set of ugly refactors planned for late Stein are
>>>> done I
>>>> may look at building some upgrade checking, once we also work out what
>>>> out upgrade checking should look like. Probably mostly checking config
>>>> changes, database migration states, and plugin compatibility.
>>>>
>>>> Adjutant already has a concept of startup checks at least, which while
>>>> not anywhere near as extensive as they should be, mostly amount to
>>>> making sure your config file looks 'mostly' sane regarding plugins
>>>> before starting up the service, and we do intend to expand on that,
>>>> plus
>>>> we can reuse a large chunk of that for upgrade checking.
>>>
>>> OK it seems there is not really any point in trying to satisfy the
>>> upgrade checkers goal for Adjutant in Stein then. Should we just
>>> abandon the change?
>>>
>>
>> Can't we just add a noop command like we are for the services that
>> don't currently need upgrade checks?
> 
> 
> I mostly was responding to this in the review itself rather than on here.
> 
> We are probably going to have reason for an upgrade check in Adjutant,
> my main gripe is, Adjutant is Django based and there isn't a good point
> in adding a separate cli when we already expose 'adjutant-api' as a
> proxy to manage.py and as such we should just register the upgrade check
> as a custom Django admin command.
> 
> More so because all of the logic needed to actually run the check in
> future will require Django settings to be configured. We don't actually
> use any oslo libraries yet so the current code for the check doesn't
> actually make sense in context.
> 
> I'm fine with a noop check, but we have to make it fit.

What I'm trying to avoid is creating any snowflake upgrade processes. It 
may not make sense for Adjutant to do this in isolation, but Adjutant 
doesn't exist in isolation. Also, if I understand correctly, you're 
proposing to add startup checks instead of upgrade checks. The downside 
I see there is that you have to have already restarted the service 
before the check runs so if there's a problem now you have downtime. 
With a standalone upgrade check you can run the check while the old 
version of the code is still running. If problems are found you fix them 
before doing the restart.

That said, I don't particularly care how the upgrade check is 
implemented. If 'adjutant-status upgrade check' just calls 'adjutant-api 
--check' or something else that returns 0 or non-0 appropriately that 
satisfies me. I don't want to cross the line into foolish consistency 
either. :-)

-Ben