Skip to content

Conversation

dusk125
Copy link

@dusk125 dusk125 commented Aug 20, 2025

No description provided.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 20, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 20, 2025

@dusk125: This pull request references CNTRLPLANE-1285 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set.

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

Copy link
Contributor

openshift-ci bot commented Aug 20, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dusk125
Once this PR has been reviewed and has the lgtm label, please assign romanbednar for approval. For more information see the 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:

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

- name: csi-node-driver-registrar
securityContext:
privileged: true
readOnlyRootFilesystem: true
Copy link
Contributor

Choose a reason for hiding this comment

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

@dobsonj
How does privileged interact with readOnlyRootFilesystem? Will this make any change since we are already running it as privileged?

@dusk125
Copy link
Author

dusk125 commented Aug 21, 2025

/hold
for proper testing in 4.21

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 21, 2025
containers:
- name: csi-liveness-probe
securityContext:
readOnlyRootFilesystem: true
Copy link

Choose a reason for hiding this comment

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

This only add the controller sidecar, did not add readOnlyRootFilesystem to driver node.
Do we need to add this parameter to drive node?

Copy link
Member

@tsmetana tsmetana Oct 1, 2025

Choose a reason for hiding this comment

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

The driver assets should get the parameter from assets/common/readOnlyRootFilesystem.yaml: all of them get patched during the final assets generation IIUC. This actually might be problematic too since we can't exclude some of the drivers in case the readOnlyRootFilesystem would break them.

Copy link
Member

Choose a reason for hiding this comment

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

I misunderstood your comment. And I think you're right -- some sidecars don't have the readOnlyFilesystem set it seems and on both node and controller.

@stephenfin
Copy link
Contributor

Potentially dumb question arising out of openshift/hypershift#6885: do we need to mount emptyDir to /tmp for all of these? I was expecting to see that here based on openshift/cluster-storage-operator#564

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 24, 2025
@dusk125
Copy link
Author

dusk125 commented Sep 24, 2025

Potentially dumb question arising out of openshift/hypershift#6885: do we need to mount emptyDir to /tmp for all of these? I was expecting to see that here based on openshift/cluster-storage-operator#564

Some need it, some don't. It really just depends on whether or not something needs to/expects to write to /tmp. I added it by default because usually it's a safe assumption that something will write to it, but in some cases it doesn't.... I should probably add it to these :)

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 10, 2025
Copy link
Contributor

openshift-ci bot commented Oct 10, 2025

@dusk125: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/smb-operator-e2e e01f79a link false /test smb-operator-e2e
ci/prow/aws-efs-cross-account-operator-e2e fb41880 link false /test aws-efs-cross-account-operator-e2e
ci/prow/e2e-azurestack-csi fb41880 link false /test e2e-azurestack-csi
ci/prow/aws-efs-operator-e2e fb41880 link true /test aws-efs-operator-e2e
ci/prow/hypershift-aws-e2e-external fb41880 link true /test hypershift-aws-e2e-external
ci/prow/e2e-azure-file-csi-extended fb41880 link false /test e2e-azure-file-csi-extended
ci/prow/hypershift-e2e-openstack-aws-csi-cinder fb41880 link true /test hypershift-e2e-openstack-aws-csi-cinder
ci/prow/e2e-openstack-cinder-csi fb41880 link true /test e2e-openstack-cinder-csi
ci/prow/e2e-openstack-manila-csi fb41880 link true /test e2e-openstack-manila-csi
ci/prow/hypershift-e2e-openstack-aws-csi-manila fb41880 link true /test hypershift-e2e-openstack-aws-csi-manila
ci/prow/aws-efs-single-zone-cross-account-operator-e2e fb41880 link true /test aws-efs-single-zone-cross-account-operator-e2e
ci/prow/aws-efs-sts-cross-account-operator-e2e fb41880 link false /test aws-efs-sts-cross-account-operator-e2e
ci/prow/aws-efs-single-zone-operator-e2e fb41880 link true /test aws-efs-single-zone-operator-e2e
ci/prow/hypershift-e2e-aks fb41880 link true /test hypershift-e2e-aks
ci/prow/e2e-azure-csi-extended fb41880 link false /test e2e-azure-csi-extended
ci/prow/e2e-azure-file-csi fb41880 link true /test e2e-azure-file-csi
ci/prow/e2e-azure fb41880 link true /test e2e-azure
ci/prow/aws-efs-single-zone-operator-e2e-extended fb41880 link false /test aws-efs-single-zone-operator-e2e-extended
ci/prow/aws-efs-operator-e2e-extended fb41880 link false /test aws-efs-operator-e2e-extended
ci/prow/e2e-openstack fb41880 link false /test e2e-openstack

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants