Skip to content

test: Add check for Openshift registry image stream import, update tests to Openshift 1.5 #6398

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

Closed

Conversation

martinpitt
Copy link
Member

@martinpitt martinpitt commented Apr 27, 2017

Add tests for importing a docker tag into an Openshift image stream, both for pulling all and for pulling selected tags only. The latter reproduces https://bugzilla.redhat.com/show_bug.cgi?id=1373332 (it still pulls all tags in the latter case), so comment out these assertions for now. (I'll look at that bug later)

Set up a local docker registry instance to avoid having to go to the network.

This requires installing the docker registry container on Openshift image, thus a new image (uploading right now). That container is ~ 33 MB, so an acceptable increase IMHO.

Making the Openshift registry accept that local docker registry requires jumping through some hoops.

Rebuilding the Openshift image now updates it from 1.4 to 1.5, which introduces some functional changes and a major regression, so this requires some test adjustments and naughty overrides.

@martinpitt martinpitt added the bot label Apr 27, 2017
@martinpitt martinpitt force-pushed the registry-import-test branch from 67c5d69 to 72b5225 Compare April 27, 2017 13:06
@martinpitt martinpitt removed the bot label Apr 27, 2017
@stefwalter
Copy link
Contributor

Unfortunately Openshift has regressed in various ways with the latest update :S

@stefwalter
Copy link
Contributor

stefwalter commented Apr 27, 2017

See #6381. Needs:

new file mode 100644
index 0000000..f006a66
--- /dev/null
+++ b/test/verify/naughty-fedora-25/6381-openshift-api-selflink-persistentvolume
@@ -0,0 +1,5 @@
+  File "/data/src/cockpit/test/verify/kubelib.py", line *, in testVolumes
+    b.wait_not_present("modal-dialog")
+*
+Error: timeout
+the server could not find the requested resource

And also s/list/watch/ in check-openshift

@martinpitt
Copy link
Member Author

Thanks Stef. I (locally still) added naughty overrides for the selfLink issue on F25 and F26 and updated the s/list/watch/. There's still a third failure:

not ok 1 testImages (__main__.TestRegistry) duration: 44s
Error: tbody[data-id='marmalade/busybee:0.x'] tr td.listing-ct-toggle is ambigous

investigating that one.

@martinpitt
Copy link
Member Author

martinpitt commented Apr 28, 2017

Indeed all image tags are now being shown twice with the new image:

duplicated-tags
This isn't the case with the current openshift image in master.

The actual images seem fine, so this sounds like some protocol change in newer openshift:

[root@f1 ~]# oc get images -n marmalade|grep busybee
sha256:03ac0a73525eaa7f1663bdff536e97ef55984b6e563f254e3ea46af69ce14f58   172.30.107.54:5000/marmalade/busybee@sha256:03ac0a73525eaa7f1663bdff536e97ef55984b6e563f254e3ea46af69ce14f58
sha256:ccd219a3f04c6f49099555236d5f0f04161e5d002b9b0fe87ce846214c3be8a6   172.30.107.54:5000/marmalade/busybee@sha256:ccd219a3f04c6f49099555236d5f0f04161e5d002b9b0fe87ce846214c3be8a6
[root@f1 ~]# docker images|grep busybee
registry:5000/marmalade/busybee              0.x                 00f017a8c2a6        7 weeks ago         1.11 MB
registry:5000/marmalade/busybee              latest              00f017a8c2a6        7 weeks ago         1.11 MB

@martinpitt
Copy link
Member Author

martinpitt commented Apr 28, 2017

The duplicated images are because imageByTag(tag) now returns two images for each tag (abridged):

{
    "/oapi/v1/images/sha256:ccd219a3f04c6f49099555236d5f0f04161e5d002b9b0fe87ce846214c3be8a6": {
        "metadata": {
            "name": "sha256:ccd219a3f04c6f49099555236d5f0f04161e5d002b9b0fe87ce846214c3be8a6",
       },
   "/oapi/v1/imagessha256%3Accd219a3f04c6f49099555236d5f0f04161e5d002b9b0fe87ce846214c3be8a6": {
        "kind": "Image",
        "apiVersion": "v1",
        "metadata": {
            "name": "sha256:ccd219a3f04c6f49099555236d5f0f04161e5d002b9b0fe87ce846214c3be8a6",
            "selfLink": "/oapi/v1/imagessha256%3Accd219a3f04c6f49099555236d5f0f04161e5d002b9b0fe87ce846214c3be8a6",
       }
}

So this apparently is yet another fallout from broken selfLinks.

This is visible with this debugging statement:

--- a/pkg/kubernetes/views/image-listing.html
+++ b/pkg/kubernetes/views/image-listing.html
@@ -38,6 +38,7 @@
             <td listing-panel kind="ImageStream" colspan="4"></td>
         </tr>
     </tbody>
+    <p ng-repeat="tag in stream.status.tags track by tag.tag">{{ tag }} imageByTag: {{ imageByTag(tag) }}</p>
     <tbody ng-repeat="tag in stream.status.tags track by tag.tag"
            ng-init="iid = sid + ':' + tag.tag" data-id="{{ iid }}"
            ng-class="{open: listing.expanded(iid), last: $last, first: $first}">

@martinpitt
Copy link
Member Author

Next one up is the same test if it hits the race of seeing the first image (only) and then stumbling over:

not ok 63 testImages (check_openshift.TestRegistry) duration: 144s
Traceback (most recent call last):
  File "./verify/check-openshift", line 381, in testImages
    b.wait_in_text(".listing-ct-body .registry-image-layers", "ADD file:")
  File "/build/cockpit/test/common/testlib.py", line 245, in wait_in_text
    return self.wait_js_func('ph_in_text', selector, text)
  File "/build/cockpit/test/common/testlib.py", line 212, in wait_js_func
    return self.phantom.wait("%s(%s)" % (func, ','.join(map(jsquote, args))))
  File "/build/cockpit/test/common/testlib.py", line 741, in <lambda>
    return lambda *args: self._invoke(name, *args)
  File "/build/cockpit/test/common/testlib.py", line 767, in _invoke
    raise Error(res['error'])
Error: timeout

@martinpitt
Copy link
Member Author

The missing "ADD file:" is because openshift 1.5's oc -o json get images does not contain the dockerImageManifest key any more, so pkg/kubernetes/scripts/images.js function handle_image() does not add that any more.

However, what confuses me is that even with the current master Openshift image, neither the visible page nor the HTML source has any trace of "ADD file:" -- it just shows the layers with their sha and size, but not the manifest:

<registry-image-layers image="image" layers="layers" class="ng-isolate-scope"><ul class="registry-image-layers"> <!-- ngRepeat: layer in layers --><li ng-repeat="layer in layers" class="hint-other"> <span title="32" class="ng-binding">32 byte</span> <p class="ng-binding">sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4</p> </li><!-- end ngRepeat: layer in layers --><li ng-repeat="layer in layers" class="hint-add"> <span title="701102" class="ng-binding">684.7 KiB</span> <p class="ng-binding">sha256:04176c8b224aa0eb9942af765f66dae866f436e75acef028fe44b8a98e045515</p> </li><!-- end ngRepeat: layer in layers --> </ul></registry-image-layers>

(there is no "ADD file" anywhere else either)

After some waiting or other external events I'm not aware of, the "ADD file" and command suddenly appear on the page, but I don't know how to reproduce this interactively as user.

As such, it appears to me that this feature is rather broken even with the current images?

@martinpitt martinpitt changed the title test: Add check for Openshift registry image stream import test: Add check for Openshift registry image stream import, update tests to Openshift 1.5 Apr 28, 2017
@stefwalter
Copy link
Contributor

As such, it appears to me that this feature is rather broken even with the current images?

It only works an an Openshift user that has the "system:admin" role. And yes it is rather broken. We should open an issue upstream and/or ask for how to replace that functionality with something that works.

@martinpitt
Copy link
Member Author

It only works an an Openshift user that has the "system:admin" role.

That's not it - our openshift test image does have that, and I can't even see it after adding a sit() right after that check and logging into the test VM in the browser -- it still shows only the "sha:" bits there.

@martinpitt
Copy link
Member Author

Wah, there is more fallout from the broken selfLinks, like "Error: [ngRepeat:dupes] Duplicates in a repeater are not allowed.".

@martinpitt
Copy link
Member Author

Wah, there is more fallout from the broken selfLinks, like Error: [ngRepeat:dupes] Duplicates in a repeater are not allowed. and .listing-ct-head li:last-child a is ambigous. IMHO this is a bit hopeless - if we add naughty overrides on all centos/fedora/rhel releases for all of these, there's not much kubernetes left to test.. Is it an option to keep the current 1.4 based image around for a while and wait for this regression to get fixed properly?

@martinpitt martinpitt added the blocked Don't land until something else happens first (see task list) label May 2, 2017
@martinpitt martinpitt self-assigned this May 2, 2017
@martinpitt martinpitt removed the blocked Don't land until something else happens first (see task list) label May 2, 2017
@martinpitt
Copy link
Member Author

In the meeting today we decided to not stall Openshift at 1.4, but update to 1.5 and add appropriate overrides.

martinpitt added 5 commits May 2, 2017 15:57
/tmp is not being cleaned out by zero-disk.setup.

Do the build in a subdirectory, which is easier to clean up and also
makes it easier to build other Docker images with different contexts.
This will be used for testing container and image stream imports into
OpenShift. We don't want tests to talk to the real hub.docker.com, so
we'll set up our own mini registry to pull from.

Openshift's image registry does not accept docker registry TLS
certificates signed by Openshift's CA by default, and there is no way to
properly configure it to do so. So rebuild the registry docker image
with the Openshift CA included in /etc/pki/.
See <openshift/origin#1753>.
Add tests for importing a docker tag into an Openshift image stream,
both for pulling all and for pulling selected tags only. The latter
reproduces https://bugzilla.redhat.com/show_bug.cgi?id=1373332 (it still
pulls all tags in the latter case), so comment out these assertions for
now.

Set up a local docker registry instance to avoid having to go to the
network.

Closes cockpit-project#6398
In "check-openshift TestOpenshift.testConnect", the new Openshift
version now responds with "cannot watch all persistentvolumes in the
cluster", instead of "list", so update the expected string.
Adjust "check-openshift TestRegistry.testImages" for not expecting the
the docker manifest commands of an image tag any more, as Openshift 1.5
does not deliver that any more as part of "get images". Now just verify
that it shows the size of the layers.

This feature is rather brittle with the current Openshift (1.4) image
even and almost impossible to see manually, so it's not a big loss of
functionality.
@martinpitt martinpitt force-pushed the registry-import-test branch from da486f5 to 48c134f Compare May 2, 2017 15:26
@martinpitt
Copy link
Member Author

I went ahead and added overrides. I'm happy to see some progress upstream on that matter at last, openshift/origin#14001 just got posted. \o/.

@martinpitt martinpitt force-pushed the registry-import-test branch from 48c134f to 18559a4 Compare May 2, 2017 19:14
@martinpitt
Copy link
Member Author

Since all of that is racy, another variant (slightly later) popped up:

Traceback (most recent call last):
  File "test/verify/check-openshift", line 535, in testImages
    b.click("tbody[data-id='marmalade/busybee:latest'] tr.listing-ct-item td.listing-ct-toggle")
  File "/home/martin/upstream/cockpit/test/common/testlib.py", line 157, in click
    self.call_js_func('ph_click', selector, force)
  File "/home/martin/upstream/cockpit/test/common/testlib.py", line 142, in call_js_func
    return self.phantom.eval("%s(%s)" % (func, ','.join(map(jsquote, args))))
  File "/home/martin/upstream/cockpit/test/common/testlib.py", line 741, in <lambda>
    return lambda *args: self._invoke(name, *args)
  File "/home/martin/upstream/cockpit/test/common/testlib.py", line 767, in _invoke
    raise Error(res['error'])
Error: tbody[data-id='marmalade/busybee:latest'] tr.listing-ct-item td.listing-ct-toggle is ambigous

I re-pushed with generalizing the pattern accordingly.

Regression in Openshift 1.5 in Fedora/CentOS/RHEL:
cockpit-project#6381
This is being fixed in openshift/origin#14001.

This breaks Cockpit's Openshift interface really hard, but nevertheless
1.5 is what is delivered to the world now.
@martinpitt
Copy link
Member Author

The fedora-26 failure is unrelated, and still affected by #6354, so I think this is ready to go now.

@stefwalter stefwalter closed this in c7be0e0 May 3, 2017
stefwalter pushed a commit that referenced this pull request May 3, 2017
This will be used for testing container and image stream imports into
OpenShift. We don't want tests to talk to the real hub.docker.com, so
we'll set up our own mini registry to pull from.

Openshift's image registry does not accept docker registry TLS
certificates signed by Openshift's CA by default, and there is no way to
properly configure it to do so. So rebuild the registry docker image
with the Openshift CA included in /etc/pki/.
See <openshift/origin#1753>.

Closes #6398
Reviewed-by: Stef Walter <[email protected]>
stefwalter pushed a commit that referenced this pull request May 3, 2017
Add tests for importing a docker tag into an Openshift image stream,
both for pulling all and for pulling selected tags only. The latter
reproduces https://bugzilla.redhat.com/show_bug.cgi?id=1373332 (it still
pulls all tags in the latter case), so comment out these assertions for
now.

Set up a local docker registry instance to avoid having to go to the
network.

Closes #6398
Reviewed-by: Stef Walter <[email protected]>
stefwalter pushed a commit that referenced this pull request May 3, 2017
In "check-openshift TestOpenshift.testConnect", the new Openshift
version now responds with "cannot watch all persistentvolumes in the
cluster", instead of "list", so update the expected string.

Closes #6398
Reviewed-by: Stef Walter <[email protected]>
stefwalter pushed a commit that referenced this pull request May 3, 2017
Adjust "check-openshift TestRegistry.testImages" for not expecting the
the docker manifest commands of an image tag any more, as Openshift 1.5
does not deliver that any more as part of "get images". Now just verify
that it shows the size of the layers.

This feature is rather brittle with the current Openshift (1.4) image
even and almost impossible to see manually, so it's not a big loss of
functionality.

Closes #6398
Reviewed-by: Stef Walter <[email protected]>
stefwalter pushed a commit that referenced this pull request May 3, 2017
Regression in Openshift 1.5 in Fedora/CentOS/RHEL:
#6381
This is being fixed in openshift/origin#14001.

This breaks Cockpit's Openshift interface really hard, but nevertheless
1.5 is what is delivered to the world now.

Closes #6398
Reviewed-by: Stef Walter <[email protected]>
@martinpitt martinpitt deleted the registry-import-test branch May 3, 2017 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants