Simone Tiraboschi has posted comments on this change. Change subject: packaging: setup: deploy Cinder and Glance Docker containers ......................................................................
Patch Set 35: (16 comments) https://gerrit.ovirt.org/#/c/39519/35/packaging/setup/ovirt_engine_setup/dockerc/constants.py File packaging/setup/ovirt_engine_setup/dockerc/constants.py: Line 30: return gettext.dgettext(message=m, domain='ovirt-engine-setup') Line 31: Line 32: Line 33: @util.export Line 34: class Const(object): > Most of these should not be Const but random Yes, right. I'll do it on a subsequent patch Line 35: C_IMAGE_RABBITMQ = 'kollaglue/centos-rdo-rabbitmq:latest' Line 36: C_NAME_RABBITMQ = 'rabbitmq' Line 37: C_IMAGE_MARIADBDATA = 'kollaglue/centos-rdo-mariadb-data:latest' Line 38: C_NAME_MARIADBDATA = 'mariadbdata' https://gerrit.ovirt.org/#/c/39519/35/packaging/setup/plugins/ovirt-engine-common/dockerc/core.py File packaging/setup/plugins/ovirt-engine-common/dockerc/core.py: Line 55: osetupcons.CoreEnv.SETUP_ATTRS_MODULES Line 56: ].append(odockerccons) Line 57: Line 58: @plugin.event( Line 59: stage=plugin.Stages.STAGE_LATE_SETUP, > Condition=? This one is indeed a bit tricky. docker is deamon, every possible operation (pulling an image, listing the locally available images, creating or removing a container...) requires a running docker daemon. So we need a running docker daemon also on customization and validation stages. The issue is that we are silently starting it and if some pre-existing container was created with 'Restart=always' it's also going to restart them. Line 60: ) Line 61: def _late_setup(self): Line 62: if not self.environment[ Line 63: osetupcons.CoreEnv.DEVELOPER_MODE https://gerrit.ovirt.org/#/c/39519/35/packaging/setup/plugins/ovirt-engine-remove/dockerc/misc.py File packaging/setup/plugins/ovirt-engine-remove/dockerc/misc.py: Line 48: self._dcli = docker.Client(base_url='unix://var/run/docker.sock') Line 49: Line 50: @plugin.event( Line 51: stage=plugin.Stages.STAGE_CUSTOMIZATION, Line 52: name=osetupcons.Stages.REMOVE_CUSTOMIZATION_COMMON, > How come we already have 3 with this name? and now you add another one? Nev The name of this bug is copy-and-paste! :-) Done Line 53: condition=lambda self: not self.environment[ Line 54: osetupcons.RemoveEnv.REMOVE_ALL Line 55: ], Line 56: ) Line 78: stage=plugin.Stages.STAGE_MISC, Line 79: condition=lambda self: ( Line 80: ( Line 81: self.environment[osetupcons.RemoveEnv.REMOVE_ALL] or Line 82: self.environment[odockerccons.RemoveEnv.REMOVE_DOCKERC] > I'd personally check only REMOVE_DOCKERC, and always set it in previous fun Done Line 83: ) and not self.environment[osetupcons.CoreEnv.DEVELOPER_MODE] Line 84: ), Line 85: ) Line 86: def _misc(self): Line 101: self.logger.info(_('Stopping {cname}').format(cname=cont)) Line 102: try: Line 103: self._dcli.stop( Line 104: container=cont, Line 105: timeout=60, > Did this ever fail for you during testing? Perhaps set a shorter timeout an After the timeout the container will be forcefully killed so we cannot loop. By the way, we are going to remove it and so we could also kill earlier. timeout (int): Timeout in seconds to wait for the container to stop before sending a SIGKILL https://docker-py.readthedocs.org/en/latest/api/ Line 106: ) Line 107: except docker.errors.APIError as ex: Line 108: if ex.response.status_code == 404: Line 109: self.logger.warning( Line 132: else: Line 133: raise ex Line 134: Line 135: if self.services.exists( Line 136: name=odockerccons.Const.DOCKER_SERVICE_NANE > Perhaps add some other condition? What if user already had it enabled and u I would probably a good idea to keep track of its initial status, or maybe it would be also better to check the list of remaining container and keep active is something else is left and disable otherwise. Line 137: ): Line 138: self.services.startup( Line 139: name=odockerccons.Const.DOCKER_SERVICE_NANE, Line 140: state=False, Line 137: ): Line 138: self.services.startup( Line 139: name=odockerccons.Const.DOCKER_SERVICE_NANE, Line 140: state=False, Line 141: ) > You don't stop? Only disable? Done Line 142: self.environment[ Line 143: odockerccons.RemoveEnv.REMOVE_DCLIST Line 144: ] = '' Line 145: https://gerrit.ovirt.org/#/c/39519/35/packaging/setup/plugins/ovirt-engine-setup/dockerc/config.py File packaging/setup/plugins/ovirt-engine-setup/dockerc/config.py: Line 65: super(Plugin, self).__init__(context=context) Line 66: self._enabled = True Line 67: self._dimages = [] Line 68: self._already_deployed_by_me = [] Line 69: self._dcli = docker.Client(base_url='unix://var/run/docker.sock') > Does this try to connect? I'd put this later, if enabled No, it doesn't till you input the first command Line 70: Line 71: @plugin.event( Line 72: stage=plugin.Stages.STAGE_INIT, Line 73: ) Line 205: ]) Line 206: self.environment[ Line 207: osetupcons.NetEnv.FIREWALLD_SUBST Line 208: ].update({ Line 209: '@OVIRT_CINDER_PORT@': odockerccons.Const.CINDER_SERVICE_PORT > Perhaps put this in env too? So user can config with answer file? Ok. I'll do with the random password patch. Line 210: }) Line 211: if self.environment[ Line 212: odockerccons.ConfigEnv.DOCKERC_GLANCE Line 213: ]: Line 219: ]) Line 220: self.environment[ Line 221: osetupcons.NetEnv.FIREWALLD_SUBST Line 222: ].update({ Line 223: '@OVIRT_GLANCE_PORT@': odockerccons.Const.GLANCE_SERVICE_PORT > same same Line 224: }) Line 225: Line 226: @plugin.event( Line 227: stage=plugin.Stages.STAGE_VALIDATION, Line 243: ) Line 244: ) Line 245: Line 246: if self.environment[ Line 247: odockerccons.RemoveEnv.REMOVE_DCLIST > Is this the list of containers deployed by us? Proposals? :-) Line 248: ]: Line 249: self._already_deployed_by_me = [ Line 250: x.strip() Line 251: for x in self.environment[ Line 261: # suffix to the container name. Line 262: if already_existing_not_by_me: Line 263: self.dialog.note( Line 264: text=_( Line 265: 'The following container was found:\n' > containers were Done Line 266: ' {found}\n' Line 267: 'Please remove or rename and ' Line 268: 'than execute the setup again\n' Line 269: ).format( Line 264: text=_( Line 265: 'The following container was found:\n' Line 266: ' {found}\n' Line 267: 'Please remove or rename and ' Line 268: 'than execute the setup again\n' > then execute Setup again Done Line 269: ).format( Line 270: found=', '.join(already_existing), Line 271: ), Line 272: ) Line 326: hostname = osetuphostname.Hostname(plugin=self) Line 327: dnsresolved = hostname.isResolvedByDNS(fqdn) Line 328: # TODO: check if we also need to force container DNS Line 329: for cont in self._dimages: Line 330: self.logger.info(_('Pulling {cname}').format(cname=cont['name'])) > No idea what this does. Is any of it configurable? Outside? E.g. what if a It downloads/updates a published docker image on your host. You could than create a container from that image. It's fully configurable to use proxy and so on but it's up to the user as is up to the user to configure yum Line 331: for line in self._dcli.pull(cont['image'], stream=True): Line 332: jline = json.loads(line) Line 333: self.logger.debug(json.dumps(jline, indent=4)) Line 334: if 'error' in jline: Line 457: """, Line 458: args=dict( Line 459: provider_id=gpUUID, Line 460: provider_name="local-glance-image-repository", Line 461: provider_description="Local Glance repository for oVirt", > perhaps localized? No idea if that's common not a bad idea Line 462: provider_url=( Line 463: 'http://' + Line 464: self.environment[osetupcons.ConfigEnv.FQDN] + Line 465: ':' + Line 497: self.logger.info(_('Enabling Docker')) Line 498: self.services.startup( Line 499: name=odockerccons.Const.DOCKER_SERVICE_NANE, Line 500: state=True, Line 501: ) > Don't start? We already started at stage STAGE_LATE_SETUP in ovirt-engine-common/dockerc/core.py Line 502: Line 503: -- To view, visit https://gerrit.ovirt.org/39519 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id192819a95343df9d9d035f8632d17ac76d2d82f Gerrit-PatchSet: 35 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Lev Veyde <lve...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
