Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apiserver: ALWAYS return apiserver.UpgradeInProgress #4629

Merged
merged 2 commits into from Mar 16, 2016
Merged

apiserver: ALWAYS return apiserver.UpgradeInProgress #4629

merged 2 commits into from Mar 16, 2016

Conversation

davecheney
Copy link
Contributor

Always, always, return apiserver.UpgradeInProgress if there is an
upgrade in progress.

(Review request: http://reviews.vapour.ws/r/4070/)

@@ -527,7 +527,7 @@ func (s *loginSuite) TestLoginValidationDuringUpgrade(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)

err = st.APICall("Client", 1, "", "DestroyModel", nil, nil)
c.Assert(err, gc.ErrorMatches, ".*upgrade in progress - Juju functionality is limited.*")
c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{Message: "upgrade in progress", Code: ""})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still isn't right, the Code should be "upgrade in progress", not the message. But at least it's coming in an rpc.RequestError.

@cherylj
Copy link

cherylj commented Mar 8, 2016

Please hold off on merging this until after we ship beta2 (should be in the next day or so).

@davecheney
Copy link
Contributor Author

Will do, that'll give me more time for manual testing.

On Wed, Mar 9, 2016 at 4:02 AM, Cheryl Jennings notifications@github.com
wrote:

Please hold off on merging this until after we ship beta2 (should be in
the next day or so).


Reply to this email directly or view it on GitHub
#4629 (comment).

@davecheney
Copy link
Contributor Author

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Mar 15, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot
Copy link
Collaborator

jujubot commented Mar 15, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/6841

Always, _always_, return apiserver.UpgradeInProgress if there is an
upgrade in progress.
UpgradeInProgressError used to be defined in apiserver/admin.go and used
in apiserver/upgrading_root.go. __BUT__, the logic that turns errors
into rpc messages is inside apiserver/common/error.go. This creates a
potential import loop as apiserver already imports common, so common
cannot import apiserver to get a type.

So, to ~~solve~~ workaround this clusterfuck, we move
UpgradeInProgressError to apiserver/params, which everyone imports and
solve the import loop.

This is _SHIT_. apiserver and apiserver/common are clearly the same
package, thay cannot function without each other and have been
bifurcated for reasons that no longer make sense.

Once this PR is landed, I will merge the two packages and then we can
define, and use UpgradeInProgressError in _one_ place, and then we can
also unexport it because things should not be checking for it explicitly
on that apiserver side of the blood/brain RPC layer.
@davecheney
Copy link
Contributor Author

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Mar 15, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

jujubot added a commit that referenced this pull request Mar 16, 2016
apiserver: ALWAYS return apiserver.UpgradeInProgress

Always, _always_, return apiserver.UpgradeInProgress if there is an
upgrade in progress.

(Review request: http://reviews.vapour.ws/r/4070/)
@jujubot jujubot merged commit fd5c08a into juju:master Mar 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants