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

Updating dashboard for 1.16 #149

Merged
merged 2 commits into from Sep 24, 2019
Merged

Updating dashboard for 1.16 #149

merged 2 commits into from Sep 24, 2019

Conversation

@hyperbolic2346
Copy link
Contributor

hyperbolic2346 commented Sep 24, 2019

The last stable dashboard release does not work with k8s 1.16. The latest
beta is now being used, which is version 2.0.0-beta4. This works with
1.16, but requires a new URL to access. The PR for the docs change is
charmed-kubernetes/kubernetes-docs#282

The last stable dashboard release does not work with k8s 1.16. The latest
beta is now being used, which is version 2.0.0-beta4. This works with
1.16, but requires a new URL to access. The PR for the docs change is
charmed-kubernetes/kubernetes-docs#282
@tvansteenburgh

This comment has been minimized.

Copy link
Contributor

tvansteenburgh commented Sep 24, 2019

Is there an LP bug for this?

@hyperbolic2346

This comment has been minimized.

Copy link
Contributor Author

hyperbolic2346 commented Sep 24, 2019

Sorry, I was working through that part.

LP bug: https://bugs.launchpad.net/cdk-addons/+bug/1845219

Tested by building cdk-addons and attaching to 1.16 edge. Had to swap the image-registry value to an empty string. Verified that make upstream-images in cdk-addons included these images.

Copy link
Contributor

tvansteenburgh left a comment

LGTM, thanks. I think we'll need to cherry-pick this to 1.16?

@tvansteenburgh

This comment has been minimized.

Copy link
Contributor

tvansteenburgh commented Sep 24, 2019

Had to swap the image-registry value to an empty string.

Because the image isn't in rocks.c.c yet?

@hyperbolic2346

This comment has been minimized.

Copy link
Contributor Author

hyperbolic2346 commented Sep 24, 2019

Had to swap the image-registry value to an empty string.

Because the image isn't in rocks.c.c yet?

Yes, but will be on next build after this merges.

@@ -126,6 +126,9 @@ def add_addon(repo, source, dest, required=True, base='cluster/addons'):
content = re.sub(r"image:\s*quay.io/",
"image: {{ registry|default('quay.io') }}/",
content)
content = re.sub(r"image:\s*kubernetesui/",
"image: {{ registry|default('kubernetesui') }}/",

This comment has been minimized.

Copy link
@kwmonroe

kwmonroe Sep 24, 2019

Contributor

Please use the host for the default registry value. I'm assuming in this case it's docker.io. We'll want to sub kubernetesui/ for rocks.c.c/kubernetesui/ just like we do for nvidia above. IOW:

-                     "image: {{ registry|default('kubernetesui') }}/",
+                     "image: {{ registry|default('docker.io') }}/kubernetesui/",

If we don't put a host here, CK will set the default registry to rocks and then try to find rocks/dashboard:v2.0.0-beta4. That won't exist since we intentionally match rocks paths to upstream paths.

@kwmonroe

This comment has been minimized.

Copy link
Contributor

kwmonroe commented Sep 24, 2019

One other thing... We're moving from kubernetes-dashboard-{{ arch }}:foo to dashboard:foo. For the uninitiated, the latter is a fat manifest:

https://hub.docker.com/u/kubernetesui/

It's a bit late in the 1.16 release to make this kind of switch. If we do this, we should do 1 of 2 (or both) things:

  • ensure cdk-addons builds for amd64 are run last; we have worked with fat manifests recently, but i'm not convinced the automated registry portion of the addons CI job will do the Right Thing. My biggest fear is something like the addons/s390x job pulling dashboard-s390x, tagging as dashboard-$wrongarch, and overwriting the amd64 portion of the dashboard manifest.

  • sed dashboard for dashboard-{{ arch }} kinda like we do for addon-resizer:

    content = content.replace(
        "kubernetesui/dashboard:v2.0.0-beta4",
        "kubernetesui/dashboard-{{ arch }}:v2.0.0-beta4"
    )

Edit to say: This might just work... but it's nerve racking for 1.16. Option #2 above is the safe thing to do.

…mages until a fat manifest is verified in rocks
@kwmonroe kwmonroe merged commit 65ab6b8 into master Sep 24, 2019
@kwmonroe kwmonroe deleted the mwilson/dashboard-1.16-fix branch Sep 24, 2019
kwmonroe added a commit that referenced this pull request Sep 24, 2019
* Updating dashboard for 1.16

The last stable dashboard release does not work with k8s 1.16. The latest
beta is now being used, which is version 2.0.0-beta4. This works with
1.16, but requires a new URL to access. The PR for the docs change is
charmed-kubernetes/kubernetes-docs#282

* set docker.io default registry; substitute image for $arch-specific images until a fat manifest is verified in rocks
kwmonroe added a commit that referenced this pull request Sep 24, 2019
* Updating dashboard for 1.16

The last stable dashboard release does not work with k8s 1.16. The latest
beta is now being used, which is version 2.0.0-beta4. This works with
1.16, but requires a new URL to access. The PR for the docs change is
charmed-kubernetes/kubernetes-docs#282

* set docker.io default registry; substitute image for $arch-specific images until a fat manifest is verified in rocks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.