Do not call "bash -c" unnecessarily as entry points #249

ralonsoh · 2024-03-14T12:23:27Z

openshift-ci · 2024-03-14T12:23:39Z

Hi @ralonsoh. Thanks for your PR.

I'm waiting for a openstack-k8s-operators member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

softwarefactory-project-zuul · 2024-03-14T12:31:44Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/9babb1ba4c57454b90662c4b937fe4e1

❌ openstack-k8s-operators-content-provider FAILURE in 6m 31s
⚠️ ovn-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

booxter · 2024-03-14T14:12:36Z

/ok-to-test

booxter · 2024-03-15T15:56:20Z

pkg/ovncontroller/daemonset.go

@@ -131,14 +131,11 @@ func DaemonSet(
 	}

 	ovnControllerCmd = []string{
-		"/bin/bash", "-c",
+		"/usr/local/bin/container-scripts/net_setup.sh && ovn-controller",


I don't think this will work, because && is handled by shell.

booxter · 2024-03-15T15:58:54Z

pkg/ovncontroller/daemonset.go

 	}
 	ovnControllerArgs = []string{
 		strings.Join(
-			append(


looks like this doesn't compile, as per CI

booxter

Doesn't compile, and I think you have to have a shell to do &&. Either we find a different way to do setup (with job / preStart hook), or have a script that would start ovn-controller for us; or do nothing.

openshift-ci · 2024-09-13T13:13:13Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ralonsoh
Once this PR has been reviewed and has the lgtm label, please ask for approval from booxter. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

softwarefactory-project-zuul · 2024-09-13T13:58:24Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/60351f5760564d2abd0ab6157bf18ff7

❌ openstack-k8s-operators-content-provider FAILURE in 7m 24s
⚠️ ovn-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

softwarefactory-project-zuul · 2024-09-16T07:08:04Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/be180f1f13d442338835c25e16a81413

❌ openstack-k8s-operators-content-provider FAILURE in 7m 07s
⚠️ ovn-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

softwarefactory-project-zuul · 2024-09-16T09:58:41Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/51e9fd937426432f92e76762194c3b1b

❌ openstack-k8s-operators-content-provider FAILURE in 8m 55s
⚠️ ovn-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

ralonsoh · 2024-09-17T06:15:20Z

@booxter @karelyatin @slawqo I think this patch is ready for review, thanks in advance.

karelyatin · 2024-09-17T07:45:55Z

@ralonsoh LGTM for the changes done here, but the linked but also mentions about bash usage in ovncontroller but i don't see any changes/comment for that either in PR or in Jira and also the comment says Closes Jira.

ralonsoh · 2024-09-17T07:56:23Z

@ralonsoh LGTM for the changes done here, but the linked but also mentions about bash usage in ovncontroller but i don't see any changes/comment for that either in PR or in Jira and also the comment says Closes Jira.

@karelyatin when the ovn-controller command [1] is executed outside the bash call, it always returns an error with the socket file. Also when dumb-init is used. The directory /run/openvswitch exists. E.g.:

ospng@r7rhoso:~/ovn-operator$ oc logs pods/$p -n kuttl3                                                                                                                                                                                                     
[dumb-init] ovn-controller --pidfile unix:/run/openvswitch/db.sock: No such file or directory

[1]https://github.com/openstack-k8s-operators/ovn-operator/blob/9bdb826b7218c51a029b8c213850022a70cea4fd/pkg/ovncontroller/daemonset.go#L39C4-L39C18

karelyatin · 2024-09-17T13:58:32Z

@ralonsoh LGTM for the changes done here, but the linked but also mentions about bash usage in ovncontroller but i don't see any changes/comment for that either in PR or in Jira and also the comment says Closes Jira.

@karelyatin when the ovn-controller command [1] is executed outside the bash call, it always returns an error with the socket file. Also when dumb-init is used. The directory /run/openvswitch exists. E.g.:
ospng@r7rhoso:~/ovn-operator$ oc logs pods/$p -n kuttl3                                                                                                                                                                                                     
[dumb-init] ovn-controller --pidfile unix:/run/openvswitch/db.sock: No such file or directory
[1]https://github.com/openstack-k8s-operators/ovn-operator/blob/9bdb826b7218c51a029b8c213850022a70cea4fd/pkg/ovncontroller/daemonset.go#L39C4-L39C18

@ralonsoh but dumb-init shouldn't be needed just for this, i did tried locally removing bash and seems to work.
https://paste.opendev.org/show/bcDn2u4DRCaMbNqnJvfk/

ralonsoh · 2024-09-18T06:01:38Z

@ralonsoh but dumb-init shouldn't be needed just for this, i did tried locally removing bash and seems to work. https://paste.opendev.org/show/bcDn2u4DRCaMbNqnJvfk/

I did something similar some PS ago [1] and it wasn't working. The problem was to divide the command string... Thanks for your help.

[1]40f649e#diff-853730e1a0e56623824bb603929544f54d838010a75b27a62b285ae738064744

softwarefactory-project-zuul · 2024-09-18T06:11:34Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/86d0858cda2a4117908ca694ce919b3c

❌ openstack-k8s-operators-content-provider FAILURE in 7m 35s
⚠️ ovn-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

Resolves: #OSPRH-680

ralonsoh · 2024-09-18T08:41:28Z

/retest-required

openshift-ci bot requested review from frenzyfriday and viroel March 14, 2024 12:23

openshift-ci bot added the needs-ok-to-test label Mar 14, 2024

openshift-ci bot added ok-to-test and removed needs-ok-to-test labels Mar 14, 2024

booxter reviewed Mar 15, 2024

View reviewed changes

booxter requested changes Mar 15, 2024

View reviewed changes

openshift-ci bot assigned booxter Mar 15, 2024

openshift-merge-robot added the needs-rebase label Apr 2, 2024

ralonsoh force-pushed the OSPRH-680 branch from 2373da2 to 7ca5785 Compare September 13, 2024 13:13

openshift-merge-robot removed the needs-rebase label Sep 13, 2024

ralonsoh force-pushed the OSPRH-680 branch from 7ca5785 to 40f649e Compare September 16, 2024 07:00

ralonsoh force-pushed the OSPRH-680 branch from 40f649e to 2d21210 Compare September 16, 2024 09:48

ralonsoh force-pushed the OSPRH-680 branch from 2d21210 to 5a0efa0 Compare September 16, 2024 15:07

ralonsoh force-pushed the OSPRH-680 branch from 5a0efa0 to 4913207 Compare September 18, 2024 06:03

Do not call "bash -c" unnecessarily as entry points

241370a

Resolves: #OSPRH-680

ralonsoh force-pushed the OSPRH-680 branch from 4913207 to 241370a Compare September 18, 2024 06:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not call "bash -c" unnecessarily as entry points #249

Do not call "bash -c" unnecessarily as entry points #249

ralonsoh commented Mar 14, 2024 •

edited by openshift-ci bot

Loading

openshift-ci bot commented Mar 14, 2024

softwarefactory-project-zuul bot commented Mar 14, 2024

booxter commented Mar 14, 2024

booxter Mar 15, 2024

booxter Mar 15, 2024

booxter left a comment

openshift-ci bot commented Sep 13, 2024

softwarefactory-project-zuul bot commented Sep 13, 2024

softwarefactory-project-zuul bot commented Sep 16, 2024

softwarefactory-project-zuul bot commented Sep 16, 2024

ralonsoh commented Sep 17, 2024

karelyatin commented Sep 17, 2024

ralonsoh commented Sep 17, 2024

karelyatin commented Sep 17, 2024 •

edited

Loading

ralonsoh commented Sep 18, 2024

softwarefactory-project-zuul bot commented Sep 18, 2024

ralonsoh commented Sep 18, 2024

Do not call "bash -c" unnecessarily as entry points #249

Are you sure you want to change the base?

Do not call "bash -c" unnecessarily as entry points #249

Conversation

ralonsoh commented Mar 14, 2024 • edited by openshift-ci bot Loading

openshift-ci bot commented Mar 14, 2024

softwarefactory-project-zuul bot commented Mar 14, 2024

booxter commented Mar 14, 2024

booxter Mar 15, 2024

Choose a reason for hiding this comment

booxter Mar 15, 2024

Choose a reason for hiding this comment

booxter left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Sep 13, 2024

softwarefactory-project-zuul bot commented Sep 13, 2024

softwarefactory-project-zuul bot commented Sep 16, 2024

softwarefactory-project-zuul bot commented Sep 16, 2024

ralonsoh commented Sep 17, 2024

karelyatin commented Sep 17, 2024

ralonsoh commented Sep 17, 2024

karelyatin commented Sep 17, 2024 • edited Loading

ralonsoh commented Sep 18, 2024

softwarefactory-project-zuul bot commented Sep 18, 2024

ralonsoh commented Sep 18, 2024

ralonsoh commented Mar 14, 2024 •

edited by openshift-ci bot

Loading

karelyatin commented Sep 17, 2024 •

edited

Loading