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

Do not run placement service as root #107

Merged

Conversation

gibizer
Copy link
Collaborator

@gibizer gibizer commented Nov 16, 2023

@openshift-ci openshift-ci bot requested review from kk7ds and mrkisaolamb November 16, 2023 14:37
@gibizer gibizer changed the title Do not run nova services as root Do not run placement service as root Nov 16, 2023
@gibizer
Copy link
Collaborator Author

gibizer commented Nov 16, 2023

/test precommit-check

   * could not run steps: step precommit-check failed: test "precommit-check" failed: could not watch pod: the pod ci-op-gids8xyf/precommit-check failed after 2m3s (failed containers: test): ContainerFailed one or more containers exited \

Copy link
Contributor

@bogdando bogdando left a comment

Choose a reason for hiding this comment

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

lgtm

@gibizer
Copy link
Collaborator Author

gibizer commented Nov 20, 2023

/hold

This should not work as kolla_set_config should fail.

Also we cannot simply switch to the placement user in the container as that user is not in the kolla group:

❯ oc rsh placement-7ccc6bc7f5-cfchj
Defaulted container "placement-api" out of: placement-api, init (init)
sh-5.1# id
uid=0(root) gid=0(root) groups=0(root)
sh-5.1# cat /etc/passwd | grep placement
placement:x:997:994:OpenStack Placement:/:/bin/bash
sh-5.1# cat /etc/group | grep kolla
kolla:x:42400:
sh-5.1# 

@gibizer
Copy link
Collaborator Author

gibizer commented Nov 20, 2023

And we have a strange dbsync command:

	DBSyncCommand = "/usr/local/bin/kolla_set_configs && su -s /bin/sh -c \"placement-manage db sync\" placement"

@gibizer
Copy link
Collaborator Author

gibizer commented Nov 20, 2023

We need a fix in the tcib image first openstack-k8s-operators/tcib#102

@gibizer
Copy link
Collaborator Author

gibizer commented Nov 22, 2023

tcib fix merged but there is still no container image built in https://quay.io/repository/podified-antelope-centos9/openstack-placement-api?tab=tags&tag=latest so we wait...

@gibizer
Copy link
Collaborator Author

gibizer commented Nov 23, 2023

container promotion is blocked on https://issues.redhat.com/browse/OSPCIX-119

Copy link
Collaborator

@SeanMooney SeanMooney left a comment

Choose a reason for hiding this comment

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

https://issues.redhat.com/browse/OSPCIX-119 is closed
and there was a successful promotion yesterday
https://review.rdoproject.org/zuul/build/97b34cff01744289b42f01944b777616
so i belive we can now proceed with this

@GIBI if you agree then feel free to drop your hold

@openshift-ci openshift-ci bot added the lgtm label Dec 5, 2023
@openshift-ci openshift-ci bot removed the lgtm label Dec 7, 2023
@gibizer
Copy link
Collaborator Author

gibizer commented Dec 7, 2023

/unhold

@gibizer
Copy link
Collaborator Author

gibizer commented Dec 7, 2023

This has the same limitation as nova-operator has. We cannot remove the usage of root from the container as kolla_set_config needs sudo.

@gibizer
Copy link
Collaborator Author

gibizer commented Dec 7, 2023

/hold
We need to wait for openstack-k8s-operators/openstack-operator#591 to pick up the new dataplane-operator that will pick up the new placement-api tcib image.

@gibizer
Copy link
Collaborator Author

gibizer commented Dec 7, 2023

/hold We need to wait for openstack-k8s-operators/openstack-operator#591 to pick up the new dataplane-operator that will pick up the new placement-api tcib image.

I mixed up this needs a new placement-operator to pick up a new placement-api tcib image. But we are in placement-operator so we don't have to wait for that.

@gibizer
Copy link
Collaborator Author

gibizer commented Dec 7, 2023

/unhold

This did not removed the root usage from the init container. We should
get rid of the init container instead. (See openstack-k8s-operators#64)

Implements: https://issues.redhat.com/browse/OSPRH-1374
@bogdando
Copy link
Contributor

bogdando commented Dec 8, 2023

/LGTM

@openshift-ci openshift-ci bot added the lgtm label Dec 8, 2023
@bogdando bogdando self-requested a review December 8, 2023 13:33
Copy link
Contributor

openshift-ci bot commented Dec 8, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bogdando, gibizer, SeanMooney

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [SeanMooney,bogdando,gibizer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 8fdff26 into openstack-k8s-operators:main Dec 8, 2023
6 checks passed
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.

3 participants