Merge lp://staging/~vila/ols-store-tests/redirection-failures into lp://staging/~ubuntuone-pqm-team/ols-store-tests/store-acceptance-tests

Proposed by Vincent Ladeuil
Status: Merged
Approved by: Vincent Ladeuil
Approved revision: 42
Merged at revision: 42
Proposed branch: lp://staging/~vila/ols-store-tests/redirection-failures
Merge into: lp://staging/~ubuntuone-pqm-team/ols-store-tests/store-acceptance-tests
Diff against target: 35 lines (+12/-2)
1 file modified
tests/api/snap/test_register_name.py (+12/-2)
To merge this branch: bzr merge lp://staging/~vila/ols-store-tests/redirection-failures
Reviewer Review Type Date Requested Status
Ricardo Kirkner (community) Approve
Review via email: mp+320166@code.staging.launchpad.net

Commit message

Fix scasnap redirection related failures.

Description of the change

https://scasnap.ols.staging.internal/ is self-signed so it can't be used as SCA_ROOT_URL.

In the long run, once it gets a proper domain, this won't be an issue.

Since these tests are focused on snap-v2, it seems more appropriate to target the new domain rather than the old and fixing redirection issues (dedicated tests would be more appropriate if we want that).

This proposal is therefore a short term solution to get the job back to green.

To post a comment you must log in.
Revision history for this message
Ricardo Kirkner (ricardokirkner) wrote :

<pindonga> vila, mhh
<pindonga> I mean... that means that the current staging server and tests only work inside the datacenter?
<pindonga> afaik .internal is not resolvable outside the internal network
<pindonga> not even sure if via vpn
<vila> pindonga: hmm, no, the tests work from outside
<vila> pindonga: what doesn't work (from inside the vpn) is to attempt to use .internal because it's self-signed so ssl barfs
<pindonga> ah, it works bc it only shows you the url in the data
<pindonga> it doesn't try to actually go to that url
<pindonga> that would fail
<vila> pindonga: the tests (as they are written right now), requires that the url in the response is myapps. yeah that
<vila> yes
<pindonga> do you know why we redirect to an internal url?
<pindonga> is it bc we don't yet have a dns name defined?
<vila> because the domain is still unknown
<vila> yes
<pindonga> k
<pindonga> I guess it's ok for now
<pindonga> though I'm not that satisfied (with the overall redirect approach) I'll +1 this mp
<vila> so the tests will fail again once we have that domain, but I prefer that over... yeah

review: Approve
Revision history for this message
Michael Nelson (michael.nelson) wrote :

FWIW, the tests aren't restricted to the datacenter - as they don't involve needing to browse the new domain. All that's being checked here is the returned URL - which since it's being processed (via proxy) on the new deployment, uses the new domain.

We now have a domain and are updating staging to use dashboard.staging.snapcraft.io, but you'll still have the issue that you're hitting myapps.developer.staging.ubuntu.com here, but the response is being served by dashboard.staging.snapcraft.io - as will the returned URL - which is correct.

At a later date, we can update these tests to run against dashboard.staging.., but right now I think they should continue to run against myapps.developer.staging and expect the api response to direct people to the new url.

Revision history for this message
Vincent Ladeuil (vila) wrote :

> At a later date, we can update these tests to run against dashboard.staging.., but right now I think they should continue to run against myapps.developer.staging and expect the api response to direct people to the new url.

Why not using the new domain right now ? (I have a branch where I just check dashboard.staging.snapcraft.io instead of scasnap.ols.staging.internal and they pass).

I'm doing a run with SCA_ROOT_URL pointed at the new domain.... and it passes.

Discussing with Shawn and Kelvin though, the consensus is to keep pointing at myapps.developer.staging so the tests can be used as a safety net for the redirection work.

See https://code.launchpad.net/~vila/ols-store-tests/new-dashboard-doamin/+merge/321039

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches