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

kubernetes: import specific tags from image repository #9682

Merged
merged 1 commit into from Aug 2, 2018

Conversation

croissanne
Copy link
Contributor

No description provided.

@martinpitt
Copy link
Member

Please update the corresponding test in kubelib.py (git grep EXFAIL) and refer to https://bugzilla.redhat.com/show_bug.cgi?id=1373332 in the commit log. Even though the test doesn't actually get that far due to the docker whitelist barrier.

@croissanne croissanne changed the title WIP kubernetes: import specific tags from image repository kubernetes: import specific tags from image repository Jul 29, 2018
@martinpitt
Copy link
Member

Semaphore fails:

> log: not ok 2 - build spec correctly, expected: {
#   "dockerImageRepository": "busybox",
#   "tags": [
#     {
#       "importPolicy": {
#         "insecure": true
#       },
#       "name": "2.5"
#     },
#     {
#       "annotations": null,
#       "from": {
#         "kind": "DockerImage",
#         "name": "busybox:latest"
#       },
#       "generation": 2,
#       "importPolicy": {
#         "insecure": true
#       },
#       "name": "latest"
#     },
#     {
#       "importPolicy": {
#         "insecure": true
#       },
#       "name": "second"
#     }
#   ]
# }, got: {
#   "dockerImageRepository": "busybox",
#   "tags": [
#     {
#       "from": {
#         "kind": "DockerImage",
#         "name": "undefined:2.5"
#       },
#       "importPolicy": {
#         "insecure": true
#       },
#       "name": "2.5"
#     },
#     {
#       "annotations": null,
#       "from": {
#         "kind": "DockerImage",
#         "name": "busybox:latest"
#       },
#       "generation": 2,
#       "importPolicy": {
#         "insecure": true
#       },
#       "name": "latest"
#     },
#     {
#       "from": {
#         "kind": "DockerImage",
#         "name": "undefined:second"
#       },
#       "importPolicy": {
#         "insecure": true
#       },
#       "name": "second"
#     }
#   ]
# }, test: buildSpec with spec, module: , source: at Object.deepEqual (http://localhost:41140/dist/kubernetes/scripts/test-tags.min.js:6743:31)
#     at http://localhost:41140/dist/kubernetes/scripts/test-tags.min.js:28:74
#     at Object.i [as invoke] (http://localhost:41140/dist/kubernetes/scripts/test-tags.min.js:958:55)
#     at e (http://localhost:41140/dist/kubernetes/scripts/test-tags.min.js:59:30)
#     at Object.<anonymous> (http://localhost:41140/dist/kubernetes/scripts/test-tags.min.js:19:34)
#     at t (http://localhost:41140/dist/kubernetes/scripts/test-tags.min.js:6269:44)
> log: 1..2
> log: cockpittest-tap-done
PASS: dist/kubernetes/scripts/test-tags.html 1 - parseSpec: parsed names correctly
FAIL: dist/kubernetes/scripts/test-tags.html 2 - build spec correctly, expected: {

Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

This uses the solution recommended by the OpenShift devs, code looks fine. I tested this on a production OpenShift instance with docker.io/prom/busybox, it works very well. Thank you!

self.assertIn("2.11", output)
# EXFAIL: https://bugzilla.redhat.com/show_bug.cgi?id=1373332
# self.assertNotIn("latest", output)
testlib.wait(lambda: all(s in self.openshift.execute('oc get imagestream --namespace=marmalade sometags') for s in ['marmalade/sometags', '2.11']))
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need to check for 'marmalade/sometags' again? the command already only queries the sometags imagestream? I. e. how does it look like when "2.11" appears, but not "marmalade/sometags"?

I don't understand at all lwhy we have to wait for this again, after the UI is already correct (in the previous paragraph). But oh well, it cannot hurt :)

Copy link
Contributor Author

@croissanne croissanne Aug 2, 2018

Choose a reason for hiding this comment

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

At first this is (sometimes) the situation right after creating:

[root@f1 ~]# oc get imagestream --namespace=marmalade sometags
NAME       DOCKER REPO                             TAGS      UPDATED
sometags   172.30.196.12:5000/marmalade/sometags

So the tags aren't present, but after waiting a second or two they are:

[root@f1 ~]# oc get imagestream --namespace=marmalade sometags
NAME       DOCKER REPO                             TAGS      UPDATED
sometags   172.30.196.12:5000/marmalade/sometags   2.11      4 seconds ago

That said we could just spin until 2.11 shows up, instead of also looking for sometags, but it can't hurt.

So the changes to the stream seem to get propagated to the cockpit ui before it's visible on the cli, weirdly enough. I have no idea why we have to wait, or why DOCKER REPO states an ip instead of localhost:5555/juggs. But they all seem to be giving this output consistently in prerelease.

Copy link
Member

Choose a reason for hiding this comment

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

Don't take the localhost:5555/juggs for granted - I wrote that test when I was young and needed the money..

Isn't localhost:5555 the registry we are importing from? Maybe this thing shows the target registry it gets imported into.

If that changes between prerelease and openshift, that might very well just be an internal implementation detail. Let's not assert anything about the repo then, just make sure that the expected tags are there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

localhost:5555 is exactly the registry we're importing from. But we are importing it into marmalade yes. So maybe as soon as you import specific tags it does something different than when you import all tags from a repo. So you're right, let's just check for tags.

I'll leave the localhost:5555/juggs in place for the other checks, since they haven't hurt anyone :D

@martinpitt martinpitt merged commit 2fae816 into cockpit-project:master Aug 2, 2018
@croissanne croissanne deleted the registry branch September 23, 2019 19:23
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.

None yet

2 participants