Skip to content

[WIP] Docker registry re-deployment tips#8666

Closed
miminar wants to merge 1 commit intoopenshift:masterfrom
miminar:registry-redeploy
Closed

[WIP] Docker registry re-deployment tips#8666
miminar wants to merge 1 commit intoopenshift:masterfrom
miminar:registry-redeploy

Conversation

@miminar
Copy link
Copy Markdown

@miminar miminar commented Apr 10, 2018

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 10, 2018
@miminar miminar force-pushed the registry-redeploy branch 2 times, most recently from 6279b96 to eca7637 Compare April 10, 2018 15:18
@miminar
Copy link
Copy Markdown
Author

miminar commented Apr 10, 2018

@dmage, @legionus, @bparees WDYT? Too verbose? Or shall I mention anything else?

@miminar
Copy link
Copy Markdown
Author

miminar commented Apr 10, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@tnozicka Any change regarding this for 3.10/3.11?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think roullout is supported since like Kubernetes 1.7

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

don't say unfortunately, we apologize for nothing! :) (but seriously, it strikes an awkward tone. just state the facts).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think roullout is supported since like Kubernetes 1.7

Well, this is what I get with oc v3.9.16-1+23bf34b-dirty:

$ oc rollout latest ds/docker-registry
error: docker-registry is not a deployment config

don't say unfortunately

My disappontment leaked into the text. I'll try to be less emotional ;-).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You said oc rollout, not oc rollout latest. DS (with rolling Update strategy set) rollout automatically on config change. You can do e.g. oc rollout undo daemonset <daemonset-name> --to-revision=<revision> to rollback. If you need to force redeployment without a change in PodSpec you need to e.g. annotate the pod template with current time until we implement rolling-restart.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@tnozicka thanks, that looks promising, I think I'll use this.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@tnozicka similar question.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not sure, but feel free to create an issue if that's not supported. oc create service and using the labels from DS should work though

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@tnozicka Is there a better command that will not cause noticeable down-time?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

https://v1-9.docs.kubernetes.io/docs/tasks/manage-daemon/update-daemon-set/

set the strategy to rollingUpdate and insert a label with current time into the pod template in the DS to force it to redeploy (rolling-restart command is planned to avoid this)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oc rollout? (won't cause downtime if the strategy is rolling)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(not applicable to daemonsets of course)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oc patch ds <ds_name> -p='{"spec":{"template":{"metadata":{"annotations":{"timestamp":"'$(date +%s%N)'"}}}}}'

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You want my contribution to become the least readable piece ever. I'm missing a note type [NerdsOnly] :-)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

But I like the hack nevertheless.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AFAIK image streams are created with an IP address and it's asynchronously replaced by the FQDN, so FQDN may appear with a little delay.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that happens only when you have master config misconfigured, at least from the issues we have been seeing with double deployments.

Copy link
Copy Markdown
Author

@miminar miminar Apr 11, 2018

Choose a reason for hiding this comment

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

The hostname specified in the configuration file takes precedence. If not present, OPENSHIFT_DEFAULT_REGISTRY env var is used. When unset, the master evaluates service's "${DOCKER_REGISTRY_SERVICE_HOST}:${DOCKER_REGISTRY_SERVICE_PORT}".

With oc cluster up, all is unset until the service exists (or maybe is up - need to verify so just the service's existence matters). Nevetheless, good point - I need to explain the empty output as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

TL;DR; if there's no output, no explicit address is configured and the service does not yet exist.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/is/are/

(docs team may have a better suggestion on cleaning up the informal language)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unclear what this means. Do you mean mean the service ip will not be changed as a result of redeploying, or is this a command to the user ("don't change the service ip when you redeploy")?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The latter. It's very easy to change the service IP and it should be avoided. I'll try to be more clear.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is confusing after having just read that the IP won't be changed. why do i need to know this if the IP isn't going to change? When do i need to know it?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If you choose to change the service IP. I'll rephrase.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

again unclear if this means "you must keep the service ip the same" or "the system will keep the service ip the same"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/though/when the DNS name is used/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if what isn't the case? if they don't match? again what if they were using a hostname?

and what if a custom IP is being used? (custom ip for what? the service ip?)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

don't you need to delete the existing "docker-registry" service first?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oc rollout? (won't cause downtime if the strategy is rolling)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(not applicable to daemonsets of course)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is this the error when the docker-registry service does not exist? That env var doesn't have anything to do w/ the service.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, this is a bit misleading. The REGISTRY_OPENSHIFT_SERVER_ADDR is the preferred env var for configuring this. If it's unset as well as some other deprecated vars with the same meaning, the registry looks for the variables inherited from the associated service ("DOCKER_REGISTRY_SERVICE_HOST").

I'm putting the explanation in there.

@miminar miminar changed the title Docker registry re-deployment tips [WIP] Docker registry re-deployment tips Apr 11, 2018
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 11, 2018
@miminar
Copy link
Copy Markdown
Author

miminar commented Apr 13, 2018

Apparently, there's a new deploy_registry.yml playbook for the re-deployment. I'll give it a shot and include it here.

Also the point of mainaining the registry's service IP is already in the docs but it's a bit outdated. I'm now merging the two.

Resolves: openshift/origin#10585

Signed-off-by: Michal Minář <miminar@redhat.com>
@miminar miminar force-pushed the registry-redeploy branch from eca7637 to ed3ad18 Compare April 16, 2018 15:42
reload their internal registry address.
+
----
$ oc env -n default ds/docker-registry "TIMESTAMP=$(date +%s%N)"
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@tnozicka is this acceptable?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Env works as well but pollutes pod's env a bit (program can see it), annotation is clearer. I'll leave the choice with you :)

I get it's shorter but people will likely just copy paste it.

@miminar
Copy link
Copy Markdown
Author

miminar commented Apr 16, 2018

Rewritten. Merged a similar section from the extended configuration. Rendered here.

I still need to include instructions for using the stand-alone registry playbook.

{"op":"add","path":"/spec/sessionAffinity","value":"ClientIP"},
{"op":"add","path":"/spec/selector","value":{"docker-registry":"default"}}
]' --local=true -o json | \
oc replace -n default -f - --force <2>
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Couldn't find out anything simpler. Originally I wanted to use oc adm registry and filter out everything but the service template. But I couldn't pull it off without using external tools like jq.

include the hostnames by which you expect the registry to be referenced.
See xref:securing_and_exposing_registry.adoc#securing-the-registry[securing
the registry] for instructions on adding hostnames to the server certificate.
-
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

TODO: restore the original

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 6, 2018
@openshift-bot
Copy link
Copy Markdown

@miminar: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@vikram-redhat
Copy link
Copy Markdown
Contributor

@miminar - is this PR still valid?

@miminar
Copy link
Copy Markdown
Author

miminar commented Aug 1, 2018

@vikram-redhat I'm afraid this needs a complete rewrite. Also with the upcoming registry operator this will become obsolete. Unfortunately, I don't have time to continue work on this now.

@miminar miminar closed this Aug 1, 2018
@vikram-redhat
Copy link
Copy Markdown
Contributor

@miminar - thanks for letting us know and closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants