Re: Do not land code on blocked branches

[John Meinel]

So I believe you can already switch to how the Juju client does it today
(call AddCharm before you call ServiceDeploy), just that we can't require
you to do so.

John
=:->

On Mon, May 4, 2015 at 7:56 PM, Richard Harding 
wrote:

> Thanks for the update. I've closed the bug for now and we'll reopen an
> issue to update to the latest api version once that's changed.
>
> On Sun, 03 May 2015, John Meinel wrote:
>
> > Just to follow up one small point, Rick. The reason Juju has to fix the
> API
> > and restore the old behavior is because even if Quickstart fixed itself,
> > anyone running an older version is going to break with a newer Juju API,
> > which is why we require API stability, and only changing things by
> bumping
> > the version. :)
> >
> > John
> > =:->
>
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Re: Do not land code on blocked branches

[Richard Harding]

Thanks for the update. I've closed the bug for now and we'll reopen an
issue to update to the latest api version once that's changed.

On Sun, 03 May 2015, John Meinel wrote:

> Just to follow up one small point, Rick. The reason Juju has to fix the API
> and restore the old behavior is because even if Quickstart fixed itself,
> anyone running an older version is going to break with a newer Juju API,
> which is why we require API stability, and only changing things by bumping
> the version. :)
> 
> John
> =:->

-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Re: Do not land code on blocked branches

[John Meinel]

Just to follow up one small point, Rick. The reason Juju has to fix the API
and restore the old behavior is because even if Quickstart fixed itself,
anyone running an older version is going to break with a newer Juju API,
which is why we require API stability, and only changing things by bumping
the version. :)

John
=:->


On Sun, May 3, 2015 at 4:38 PM, Ian Booth  wrote:

> My email was poorly worded, sorry. It's main purpose was to reply the the
> email
> from QA to let the QA folks know that an incompatibility was discovered
> which
> explains the CI test failures holding up the 1.24 release, and that a
> solution
> was in progress.
>
> John's analysis is correct. I am almost done restoring the Juju 1.16
> behaviour
> to reinstate compatibility with quickstart as it stands today. Any
> quickstart
> changes are not urgent therefore. The root cause is that Juju 1.16
> behaviour was
> removed and even though Juju clients are fine with this, existing external
> clients may inadvertently be relying on such deprecated behaviour.
>
> The difficulty is that right up until 1.18, the first point from which we
> were
> required to retain backwards compatibility "forever", quite a lot of
> functionality was deprecated. It's hard to know which external (non Juju)
> clients depend on such behaviour. That's why we have CI tests for the
> important
> clients like quickstart and deployer. So in this case, the CI tests have
> done
> their job :-)
>
> On 03/05/15 22:11, John Meinel wrote:
> > Just going off the bits that Ian pointed to, the section of code was if
> you
> > called ServiceDeploy with a CharmStore URL (eg cs:mysql) but you had not
> > already called AddCharm.
> >
> > The "juju" cli client already knows to call Client.AddCharm with the
> given
> > URL, whereas the internal api/client/client.go does a double check if it
> > gets called with a charm URL that isn't already in state.
> >
> > Now, I don't know how Quickstart would be triggering
> > apiserver/client/client.go
> > The error here: in traceback looks like:
> > connecting to wss://
> > 52.6.157.186:17070/environment/47724da5-9b38-4141-8f92-03d8f4225de9/api
> > environment type: ec2
> > bootstrap node series: trusty
> > charm URL: cs:trusty/juju-gui-27
> > requesting juju-gui deployment
> > juju-quickstart: error: bad API response: charm "cs:trusty/juju-gui-27"
> not
> > found
> > 2015-05-01 18:28:59 ERROR Command '('juju', '--show-log', 'quickstart',
> > '-e', 'aws-quickstart-bundle', '--constraints', 'mem=2G', '--no-browser',
> > '/var/lib/jenkins/repository/landscape-scalable.yaml')' returned non-zero
> > exit status 1
> > Traceback (most recent call last):
> >   File "/var/lib/jenkins/juju-ci-tools/quickstart_deploy.py", line 51, in
> > run
> > for step in self.iter_steps():
> >   File "/var/lib/jenkins/juju-ci-tools/quickstart_deploy.py", line 70, in
> > iter_steps
> > self.client.quickstart(self.bundle_path)
> >   File "/mnt/jenkinshome/juju-ci-tools/jujupy.py", line 335, in
> quickstart
> > self.juju('quickstart', args, self.env.needs_sudo())
> >   File "/mnt/jenkinshome/juju-ci-tools/jujupy.py", line 294, in juju
> > return subprocess.check_call(args, env=env)
> >   File "/usr/lib/python2.7/subprocess.py", line 511, in check_call
> > raise CalledProcessError(retcode, cmd)
> >
> >
> > So it would appear that we had code to allow users to call
> > Client.ServiceDeploy(cs:mysql) and the server would lookup the charm and
> > deploy it for the user, but we stopped doing that as a Juju CLI since
> 1.16.
> >
> > However, I think this is *our* bad because this is a very important
> client
> > (quickstart and probably others) that has been relying on this behavior
> in
> > all our recent releases.
> >
> > Compat with juju-cli != compatibility with Juju API users.
> >
> > AFAIK we don't have a great way to respond to clients that behavior is
> > deprecated, but we can bump the Version of the API and change the
> behavior.
> > We definitely should have done that here rather than just remove the
> > behavior.
> >
> > John
> > =:->
> >
> > On Sun, May 3, 2015 at 3:59 PM, Richard Harding <
> rick.hard...@canonical.com>
> > wrote:
> >
> >> On Sun, 03 May 2015, Ian Booth wrote:
> >>
> 
>  Curtis has filed three new bugs for these so far, and there appears to
>  be one or two more to come:
> 
>  
> >>>
> >>> The issue here is that quickstart is relying on Juju 1.16 behaviour.
> >> There was a
> >>> block of code with a comment:
> >>>
> >>> // Remove this whole if block when 1.16 compatibility is dropped.
> >>>
> >>> The code block was removed because 1.18 was our minimum compatibility
> >> version.
> >>> But it seems we have to restore the 1.16 behaviour. Note that this is
> >> not an
> >>> upgraded environment where we need to retain compatibility with older
> >>> deployments. It is a fresh 1.24 install which should be able to rely on
> >> 1.18 and
> >>> later behaviour 

Re: Do not land code on blocked branches

[Ian Booth]

My email was poorly worded, sorry. It's main purpose was to reply the the email
from QA to let the QA folks know that an incompatibility was discovered which
explains the CI test failures holding up the 1.24 release, and that a solution
was in progress.

John's analysis is correct. I am almost done restoring the Juju 1.16 behaviour
to reinstate compatibility with quickstart as it stands today. Any quickstart
changes are not urgent therefore. The root cause is that Juju 1.16 behaviour was
removed and even though Juju clients are fine with this, existing external
clients may inadvertently be relying on such deprecated behaviour.

The difficulty is that right up until 1.18, the first point from which we were
required to retain backwards compatibility "forever", quite a lot of
functionality was deprecated. It's hard to know which external (non Juju)
clients depend on such behaviour. That's why we have CI tests for the important
clients like quickstart and deployer. So in this case, the CI tests have done
their job :-)

On 03/05/15 22:11, John Meinel wrote:
> Just going off the bits that Ian pointed to, the section of code was if you
> called ServiceDeploy with a CharmStore URL (eg cs:mysql) but you had not
> already called AddCharm.
> 
> The "juju" cli client already knows to call Client.AddCharm with the given
> URL, whereas the internal api/client/client.go does a double check if it
> gets called with a charm URL that isn't already in state.
> 
> Now, I don't know how Quickstart would be triggering
> apiserver/client/client.go
> The error here: in traceback looks like:
> connecting to wss://
> 52.6.157.186:17070/environment/47724da5-9b38-4141-8f92-03d8f4225de9/api
> environment type: ec2
> bootstrap node series: trusty
> charm URL: cs:trusty/juju-gui-27
> requesting juju-gui deployment
> juju-quickstart: error: bad API response: charm "cs:trusty/juju-gui-27" not
> found
> 2015-05-01 18:28:59 ERROR Command '('juju', '--show-log', 'quickstart',
> '-e', 'aws-quickstart-bundle', '--constraints', 'mem=2G', '--no-browser',
> '/var/lib/jenkins/repository/landscape-scalable.yaml')' returned non-zero
> exit status 1
> Traceback (most recent call last):
>   File "/var/lib/jenkins/juju-ci-tools/quickstart_deploy.py", line 51, in
> run
> for step in self.iter_steps():
>   File "/var/lib/jenkins/juju-ci-tools/quickstart_deploy.py", line 70, in
> iter_steps
> self.client.quickstart(self.bundle_path)
>   File "/mnt/jenkinshome/juju-ci-tools/jujupy.py", line 335, in quickstart
> self.juju('quickstart', args, self.env.needs_sudo())
>   File "/mnt/jenkinshome/juju-ci-tools/jujupy.py", line 294, in juju
> return subprocess.check_call(args, env=env)
>   File "/usr/lib/python2.7/subprocess.py", line 511, in check_call
> raise CalledProcessError(retcode, cmd)
> 
> 
> So it would appear that we had code to allow users to call
> Client.ServiceDeploy(cs:mysql) and the server would lookup the charm and
> deploy it for the user, but we stopped doing that as a Juju CLI since 1.16.
> 
> However, I think this is *our* bad because this is a very important client
> (quickstart and probably others) that has been relying on this behavior in
> all our recent releases.
> 
> Compat with juju-cli != compatibility with Juju API users.
> 
> AFAIK we don't have a great way to respond to clients that behavior is
> deprecated, but we can bump the Version of the API and change the behavior.
> We definitely should have done that here rather than just remove the
> behavior.
> 
> John
> =:->
> 
> On Sun, May 3, 2015 at 3:59 PM, Richard Harding 
> wrote:
> 
>> On Sun, 03 May 2015, Ian Booth wrote:
>>

 Curtis has filed three new bugs for these so far, and there appears to
 be one or two more to come:

 
>>>
>>> The issue here is that quickstart is relying on Juju 1.16 behaviour.
>> There was a
>>> block of code with a comment:
>>>
>>> // Remove this whole if block when 1.16 compatibility is dropped.
>>>
>>> The code block was removed because 1.18 was our minimum compatibility
>> version.
>>> But it seems we have to restore the 1.16 behaviour. Note that this is
>> not an
>>> upgraded environment where we need to retain compatibility with older
>>> deployments. It is a fresh 1.24 install which should be able to rely on
>> 1.18 and
>>> later behaviour only.
>>
>> Ian, can you be more specific on the chunk of code that was removed or
>> branch I can look at for this? I'll happily file a bug and update
>> quickstart, we just need to know what's changed there. Having a branch in
>> hand or a bug will assist us in getting that updated as fast as possible.
>>
>> In searching through the quickstart code there's no hard requirement or
>> notes on 1.16.
>>
>> Thanks
>>
>> --
>>
>> Rick Harding
>>
>> Juju UI Engineering
>> https://launchpad.net/~rharding
>> @mitechie
>>
>> --
>> Juju-dev mailing list
>> Juju-dev@lists.ubuntu.com
>> Modify settings or unsubscribe at:
>> https://

Re: Do not land code on blocked branches

[Richard Harding]

Thanks for the breakdown John. I've filed the bug on quickstart [1] and
I'll get our guys looking at it on Monday. For now, since the team has
diagnosed this is indeed an issue in quickstart relying on deprecated
behavior and not an unexpected API compatibility issue, I'd suggest that we
make the quickstart tests non-voting until we can provide an updated
quickstart version. This should help unblock core and give us time to get a
new release out.

1: https://bugs.launchpad.net/juju-quickstart/+bug/1451182

Rick

On Sun, 03 May 2015, John Meinel wrote:

> Just going off the bits that Ian pointed to, the section of code was if you
> called ServiceDeploy with a CharmStore URL (eg cs:mysql) but you had not
> already called AddCharm.
> 
> The "juju" cli client already knows to call Client.AddCharm with the given
> URL, whereas the internal api/client/client.go does a double check if it
> gets called with a charm URL that isn't already in state.
> 
> Now, I don't know how Quickstart would be triggering
> apiserver/client/client.go
> The error here: in traceback looks like:
> connecting to wss://
> 52.6.157.186:17070/environment/47724da5-9b38-4141-8f92-03d8f4225de9/api
> environment type: ec2
> bootstrap node series: trusty
> charm URL: cs:trusty/juju-gui-27
> requesting juju-gui deployment
> juju-quickstart: error: bad API response: charm "cs:trusty/juju-gui-27" not
> found
> 2015-05-01 18:28:59 ERROR Command '('juju', '--show-log', 'quickstart',
> '-e', 'aws-quickstart-bundle', '--constraints', 'mem=2G', '--no-browser',
> '/var/lib/jenkins/repository/landscape-scalable.yaml')' returned non-zero
> exit status 1
> Traceback (most recent call last):
>   File "/var/lib/jenkins/juju-ci-tools/quickstart_deploy.py", line 51, in
> run
> for step in self.iter_steps():
>   File "/var/lib/jenkins/juju-ci-tools/quickstart_deploy.py", line 70, in
> iter_steps
> self.client.quickstart(self.bundle_path)
>   File "/mnt/jenkinshome/juju-ci-tools/jujupy.py", line 335, in quickstart
> self.juju('quickstart', args, self.env.needs_sudo())
>   File "/mnt/jenkinshome/juju-ci-tools/jujupy.py", line 294, in juju
> return subprocess.check_call(args, env=env)
>   File "/usr/lib/python2.7/subprocess.py", line 511, in check_call
> raise CalledProcessError(retcode, cmd)
> 
> 
> So it would appear that we had code to allow users to call
> Client.ServiceDeploy(cs:mysql) and the server would lookup the charm and
> deploy it for the user, but we stopped doing that as a Juju CLI since 1.16.
> 
> However, I think this is *our* bad because this is a very important client
> (quickstart and probably others) that has been relying on this behavior in
> all our recent releases.
> 
> Compat with juju-cli != compatibility with Juju API users.
> 
> AFAIK we don't have a great way to respond to clients that behavior is
> deprecated, but we can bump the Version of the API and change the behavior.
> We definitely should have done that here rather than just remove the
> behavior.
> 
> John
> =:->
> 
> On Sun, May 3, 2015 at 3:59 PM, Richard Harding 
> wrote:
> 
> > On Sun, 03 May 2015, Ian Booth wrote:
> >
> > > >
> > > > Curtis has filed three new bugs for these so far, and there appears to
> > > > be one or two more to come:
> > > >
> > > > 
> > >
> > > The issue here is that quickstart is relying on Juju 1.16 behaviour.
> > There was a
> > > block of code with a comment:
> > >
> > > // Remove this whole if block when 1.16 compatibility is dropped.
> > >
> > > The code block was removed because 1.18 was our minimum compatibility
> > version.
> > > But it seems we have to restore the 1.16 behaviour. Note that this is
> > not an
> > > upgraded environment where we need to retain compatibility with older
> > > deployments. It is a fresh 1.24 install which should be able to rely on
> > 1.18 and
> > > later behaviour only.
> >
> > Ian, can you be more specific on the chunk of code that was removed or
> > branch I can look at for this? I'll happily file a bug and update
> > quickstart, we just need to know what's changed there. Having a branch in
> > hand or a bug will assist us in getting that updated as fast as possible.
> >
> > In searching through the quickstart code there's no hard requirement or
> > notes on 1.16.
> >
> > Thanks
> >
> > --
> >
> > Rick Harding
> >
> > Juju UI Engineering
> > https://launchpad.net/~rharding
> > @mitechie
> >
> > --
> > Juju-dev mailing list
> > Juju-dev@lists.ubuntu.com
> > Modify settings or unsubscribe at:
> > https://lists.ubuntu.com/mailman/listinfo/juju-dev
> >

-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Re: Do not land code on blocked branches

[John Meinel]

Just going off the bits that Ian pointed to, the section of code was if you
called ServiceDeploy with a CharmStore URL (eg cs:mysql) but you had not
already called AddCharm.

The "juju" cli client already knows to call Client.AddCharm with the given
URL, whereas the internal api/client/client.go does a double check if it
gets called with a charm URL that isn't already in state.

Now, I don't know how Quickstart would be triggering
apiserver/client/client.go
The error here: in traceback looks like:
connecting to wss://
52.6.157.186:17070/environment/47724da5-9b38-4141-8f92-03d8f4225de9/api
environment type: ec2
bootstrap node series: trusty
charm URL: cs:trusty/juju-gui-27
requesting juju-gui deployment
juju-quickstart: error: bad API response: charm "cs:trusty/juju-gui-27" not
found
2015-05-01 18:28:59 ERROR Command '('juju', '--show-log', 'quickstart',
'-e', 'aws-quickstart-bundle', '--constraints', 'mem=2G', '--no-browser',
'/var/lib/jenkins/repository/landscape-scalable.yaml')' returned non-zero
exit status 1
Traceback (most recent call last):
  File "/var/lib/jenkins/juju-ci-tools/quickstart_deploy.py", line 51, in
run
for step in self.iter_steps():
  File "/var/lib/jenkins/juju-ci-tools/quickstart_deploy.py", line 70, in
iter_steps
self.client.quickstart(self.bundle_path)
  File "/mnt/jenkinshome/juju-ci-tools/jujupy.py", line 335, in quickstart
self.juju('quickstart', args, self.env.needs_sudo())
  File "/mnt/jenkinshome/juju-ci-tools/jujupy.py", line 294, in juju
return subprocess.check_call(args, env=env)
  File "/usr/lib/python2.7/subprocess.py", line 511, in check_call
raise CalledProcessError(retcode, cmd)


So it would appear that we had code to allow users to call
Client.ServiceDeploy(cs:mysql) and the server would lookup the charm and
deploy it for the user, but we stopped doing that as a Juju CLI since 1.16.

However, I think this is *our* bad because this is a very important client
(quickstart and probably others) that has been relying on this behavior in
all our recent releases.

Compat with juju-cli != compatibility with Juju API users.

AFAIK we don't have a great way to respond to clients that behavior is
deprecated, but we can bump the Version of the API and change the behavior.
We definitely should have done that here rather than just remove the
behavior.

John
=:->

On Sun, May 3, 2015 at 3:59 PM, Richard Harding 
wrote:

> On Sun, 03 May 2015, Ian Booth wrote:
>
> > >
> > > Curtis has filed three new bugs for these so far, and there appears to
> > > be one or two more to come:
> > >
> > > 
> >
> > The issue here is that quickstart is relying on Juju 1.16 behaviour.
> There was a
> > block of code with a comment:
> >
> > // Remove this whole if block when 1.16 compatibility is dropped.
> >
> > The code block was removed because 1.18 was our minimum compatibility
> version.
> > But it seems we have to restore the 1.16 behaviour. Note that this is
> not an
> > upgraded environment where we need to retain compatibility with older
> > deployments. It is a fresh 1.24 install which should be able to rely on
> 1.18 and
> > later behaviour only.
>
> Ian, can you be more specific on the chunk of code that was removed or
> branch I can look at for this? I'll happily file a bug and update
> quickstart, we just need to know what's changed there. Having a branch in
> hand or a bug will assist us in getting that updated as fast as possible.
>
> In searching through the quickstart code there's no hard requirement or
> notes on 1.16.
>
> Thanks
>
> --
>
> Rick Harding
>
> Juju UI Engineering
> https://launchpad.net/~rharding
> @mitechie
>
> --
> Juju-dev mailing list
> Juju-dev@lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Re: Do not land code on blocked branches

[Richard Harding]

On Sun, 03 May 2015, Ian Booth wrote:

> >
> > Curtis has filed three new bugs for these so far, and there appears to
> > be one or two more to come:
> >
> > 
>
> The issue here is that quickstart is relying on Juju 1.16 behaviour. There 
> was a
> block of code with a comment:
>
> // Remove this whole if block when 1.16 compatibility is dropped.
>
> The code block was removed because 1.18 was our minimum compatibility version.
> But it seems we have to restore the 1.16 behaviour. Note that this is not an
> upgraded environment where we need to retain compatibility with older
> deployments. It is a fresh 1.24 install which should be able to rely on 1.18 
> and
> later behaviour only.

Ian, can you be more specific on the chunk of code that was removed or
branch I can look at for this? I'll happily file a bug and update
quickstart, we just need to know what's changed there. Having a branch in
hand or a bug will assist us in getting that updated as fast as possible.

In searching through the quickstart code there's no hard requirement or
notes on 1.16.

Thanks

--

Rick Harding

Juju UI Engineering
https://launchpad.net/~rharding
@mitechie

-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Re: Do not land code on blocked branches

[Ian Booth]

> 
> Curtis has filed three new bugs for these so far, and there appears to
> be one or two more to come:
> 
> 

The issue here is that quickstart is relying on Juju 1.16 behaviour. There was a
block of code with a comment:

// Remove this whole if block when 1.16 compatibility is dropped.

The code block was removed because 1.18 was our minimum compatibility version.
But it seems we have to restore the 1.16 behaviour. Note that this is not an
upgraded environment where we need to retain compatibility with older
deployments. It is a fresh 1.24 install which should be able to rely on 1.18 and
later behaviour only.



-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Re: Do not land code on blocked branches

[Ian Booth]

> Curtis has filed three new bugs for these so far, and there appears to
> be one or two more to come:
> 
> 
> 


The quickstart bugs have two root causes. A fix has already landed for one
issue. A fix for the other one is close to landing.

-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Do not land code on blocked branches

[Martin Packman]

It has been great that everyone has been diving in and tackling the
backlog of bugs over the last week. However, please only merge pull
requests that are actually fixing bugs blocking that branch.

In particular, the JFDI instruction to the lander is an escape hatch
to allow for merging in extraordinary circumstances, and is not for
landing non-blocking bug fixes.

If you think the bug your pr is fixing is actually a critical blocker,
but CI is preventing it landing you should first:

* Check if the launchpad bug is correctly targeted to the series you
are landing on.
* Ask Alexis, Wes, or your team lead to escalate the bug if it's not
marked as blocking.

In the last three days, master has gone from just two failing test
cases, up to ten:




CI and the QA team has been busy with releases for the 1.22 and 1.23
branches, adding a large number of regressions to master over this
time period is unhelpful.

Curtis has filed three new bugs for these so far, and there appears to
be one or two more to come:





What I wanted to do was back out Nate's branch as the likely culprit
for the windows test suite regression, but with 13 other branches
landed since then that's no longer a straightforward inference.

Martin

-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev