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.

Sign up for GitHub

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

Jump to bottom

Run httpd using kolla #620

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Run httpd using kolla #620

merged 1 commit into from
Sep 10, 2024

Conversation

fmount
Copy link
Contributor

@fmount fmount commented Sep 9, 2024 •
edited
Loading

Instead of running the httpd -DFOREGROUND command as entry point for the -httpd sidecar container, this change moves the file copy and deployment logic to kolla.
This is a requirement to not run the container as root user, because kolla helps to apply the right permissions to the config files (and pid) used by the process.
The switch from root user to GlanceUID (already present as const) will be part of a different patch, which is already in progress (#610)

Jira: https://issues.redhat.com/browse/OSPRH-10040
Jira: https://issues.redhat.com/browse/OSPRH-10143

@fmount fmount requested a review from konan-abhi September 9, 2024 20:39
@fmount fmount requested review from abays and removed request for viroel and frenzyfriday September 9, 2024 20:39
@openshift-ci openshift-ci bot added the approved label Sep 9, 2024
@fmount
Copy link
Contributor Author

fmount commented Sep 9, 2024

@maximsava12 FYI

@fmount fmount force-pushed the scc-httpd branch 3 times, most recently from 727a145 to bb7bead Compare September 10, 2024 06:54
abays
abays reviewed Sep 10, 2024
View reviewed changes
Copy link
Contributor

@abays abays left a comment

Choose a reason for hiding this comment

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

I don't know much about kolla, but so far the changes look reasonable to me

@fmount
Copy link
Contributor Author

fmount commented Sep 10, 2024

I don't know much about kolla, but so far the changes look reasonable to me

Yeah, the goal is to have flexibility in setting permissions to directories, and use the builtin scripts to copy files. The next step, after this patch lands (I'm going to fix kuttl), is to add the appropriate securityContext to the httpd sidecar and run the process as Glance user (provided by kolla) instead of root.
I'm trying to do smaller and focused changes instead of a big messy patch.

@fmount fmount force-pushed the scc-httpd branch 2 times, most recently from d28ca0a to 9e6cd1a Compare September 10, 2024 09:56
@fmount
Run httpd using kolla
9e6cd1a 
Instead of running the httpd -DFOREGROUND command as entrypoint for the
-httpd sidecar container, this change moves the file copy and deployment
logic to kolla.
This is a requirement to not run the container as root user, because
kolla helps to apply the right permissions to the config files (and pid)
used by the process.
The switch from root user to GlanceUID (already present as const) will
be part of a different patch.

Signed-off-by: Francesco Pantano <[email protected]>
@fmount
Copy link
Contributor Author

fmount commented Sep 10, 2024

Looks good and I see the expected Mounts [1]

[1] https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openstack-k8s-operators_glance-operator/620/pull-ci-openstack-k8s-operators-glance-operator-main-glance-operator-build-deploy-tempest/1833444536118939648/artifacts/glance-operator-build-deploy-tempest/openstack-k8s-operators-gather/artifacts/must-gather/quay-io-openstack-k8s-operators-openstack-must-gather-sha256-3d1f620ae3617fbd6371f55aa948b34e9daf31a461d3c557cf1a7c2db38d0385/namespaces/openstack/pods/glance-default-external-api-0/glance-default-external-api-0-describe

@fmount fmount requested a review from abays September 10, 2024 11:43
abays
abays approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@abays abays left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Sep 10, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Sep 10, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abays, fmount

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:

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 5a518ee into openstack-k8s-operators:main Sep 10, 2024
7 checks passed
@fmount fmount mentioned this pull request Sep 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abays abays abays approved these changes

@konan-abhi konan-abhi Awaiting requested review from konan-abhi

Assignees

@abays abays

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fmount @abays