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

replace docker by container in variables #4002

Closed
wants to merge 3 commits into from

Conversation

dsavineau
Copy link
Contributor

Most of the container variables are still using the docker word even
if it's possible to use podman.
This commits aims to replace the docker word by container to make the
variable name more generic.
There's still some docker specific variables (like
ceph_docker_version).

Resolves: #3532

Signed-off-by: Dimitri Savineau dsavinea@redhat.com

@dsavineau
Copy link
Contributor Author

@guits There's still ceph_docker_on_openstack and docker variables.
I don't know what is the meaning for the first one
I thought the second one was replaced by containerized_deployment
Any thoughts ?

@guits
Copy link
Collaborator

guits commented May 20, 2019

@dsavineau ceph_docker_on_openstack is a leftover, docker won't have any impact on the playbook itself, it is something used only for the CI.

@dsavineau dsavineau removed the DNM Do NOT merge label May 21, 2019
Copy link
Collaborator

@guits guits left a comment

Choose a reason for hiding this comment

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

I think first and last commits should be squashed

@dsavineau
Copy link
Contributor Author

@gfidente @fultonj What are the impact of this PR on TripleO ?

@dsavineau dsavineau force-pushed the ceph_docker_variable branch 2 times, most recently from 87b9e6c to 002716a Compare May 28, 2019 19:54
@gfidente
Copy link
Contributor

not a lot, but yes [1], thanks for pinging!

  1. https://review.opendev.org/#/c/661907/

@dsavineau dsavineau force-pushed the ceph_docker_variable branch 4 times, most recently from c189d0d to 06f3154 Compare August 1, 2019 18:55
@gfidente
Copy link
Contributor

are you guys moving ahead with this and backport it to stable-4 ?

openstack-gerrit pushed a commit to openstack-archive/tripleo-heat-templates that referenced this pull request Aug 30, 2019
Since ceph-ansible doesn't support yet the container
generic variables already merged here, this change
restores the docker registry/image_tag/container values
to pull the right container from the registry.
This change can be reverted when the ceph-ansible PR
will be merged.

ceph-ansible PR: ceph/ceph-ansible#4002

Change-Id: I5185371e92e7ea10489c7abfa89a24b12d67220b
openstack-gerrit pushed a commit to openstack/openstack that referenced this pull request Aug 30, 2019
* Update tripleo-heat-templates from branch 'master'
  - Merge "Restore docker variabes in ceph-base"
  - Restore docker variabes in ceph-base
    
    Since ceph-ansible doesn't support yet the container
    generic variables already merged here, this change
    restores the docker registry/image_tag/container values
    to pull the right container from the registry.
    This change can be reverted when the ceph-ansible PR
    will be merged.
    
    ceph-ansible PR: ceph/ceph-ansible#4002
    
    Change-Id: I5185371e92e7ea10489c7abfa89a24b12d67220b
@stale
Copy link

stale bot commented Oct 1, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Oct 1, 2019
@gfidente
Copy link
Contributor

gfidente commented Oct 2, 2019

@dsavineau I thought we wanted this one?

@stale stale bot removed the wontfix label Oct 2, 2019
@dsavineau
Copy link
Contributor Author

@gfidente yes we do. We just need some to to rebase et fix the conflict on that PR

@stale
Copy link

stale bot commented Nov 1, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@dsavineau
Copy link
Contributor Author

jenkins test centos-container-all_daemons

@dsavineau dsavineau added this to the v5.0 milestone Dec 2, 2019
@dsavineau
Copy link
Contributor Author

jenkins test centos-non_container-switch_to_containers

@dsavineau
Copy link
Contributor Author

jenkins test centos-container-update

guits
guits previously approved these changes Dec 3, 2019
@guits
Copy link
Collaborator

guits commented Dec 3, 2019

jenkins test centos-non_container-switch_to_containers

@dsavineau dsavineau force-pushed the ceph_docker_variable branch 2 times, most recently from 61a06ba to 70c9a52 Compare December 4, 2019 20:00
@dsavineau dsavineau force-pushed the ceph_docker_variable branch 2 times, most recently from e3b6218 to d101bbb Compare April 3, 2020 17:48
@dsavineau dsavineau requested a review from guits April 3, 2020 18:38
Most of the container variables are still using the docker word even
if it's possible to use podman.
This commits aims to replace the docker word by container to make the
variable name more generic.
There's still some docker specific variables (like
ceph_docker_version).

Resolves: ceph#3532

Signed-off-by: Dimitri Savineau <dsavinea@redhat.com>
This task just doesn't work when using the variable
ceph_docker_enable_centos_extra_repo to true.

fatal: [xxx]; FAILED! => {"changed": false, "msg": "Parameter
'baseurl', 'metalink' or 'mirrorlist' is required."}

The CentOS extras repository is enabled by default so it's pretty
safe to remove this task and the associated variable.

Signed-off-by: Dimitri Savineau <dsavinea@redhat.com>
This variable is a leftover and is unused. We can remove it.

Signed-off-by: Dimitri Savineau <dsavinea@redhat.com>
@dsavineau
Copy link
Contributor Author

jenkins test centos-non_container-external_clients

1 similar comment
@dsavineau
Copy link
Contributor Author

jenkins test centos-non_container-external_clients

@dsavineau
Copy link
Contributor Author

jenkins test centos-non_container-all_daemons

@guits
Copy link
Collaborator

guits commented Apr 7, 2020

jenkins test centos-non_container-switch_to_containers

@dsavineau
Copy link
Contributor Author

@guits the switch job failure is valid and I need to update the tests configuration for that

@dsavineau dsavineau closed this Jun 30, 2020
@dsavineau dsavineau deleted the ceph_docker_variable branch June 30, 2020 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

replace "docker" to "container" in variables
3 participants