[release-1.26] OCPBUGS-45908: Only restore container if all bind mounts are defined #8803

kwilczynski · 2024-12-02T07:53:45Z

This is a manual cherry-pick of commit 429ef7c

commit 429ef7c366406280cfa3daa15812d14b22953a7a
Author: Adrian Reber <[email protected]>
Date:   Mon Sep 30 23:37:13 2024

    Only restore container if all bind mounts are defined
    
    To avoid the situation where a container that is restored via a registry
    mounts unexpected host paths into the container, this changes the
    restore behaviour of CRI-O.
    
    Previously all bind mounted paths in the original container which were
    defined for example like this:
    
        volumeMounts:
        - mountPath: /data
          name: data-volume
      volumes:
      - name: data-volume
        hostPath:
          path: /srv/container/data
    
    Were automatically mounted in the restored container without and
    definition necessary. This lead to the situation that the user does not
    know which path will be mounted if starting a restored container.
    
    Now CRI-O will refuse to restore a container if not all bind mounts are
    defined via the CRI CreateContainer RPC in the CreateContainerRequest
    message.
    
    CRI-O will now return an error that will look something likes this:
    
    Error: the container to restore (7f...be) expects following bind mounts defined (/data,/data2)
    
    Now the user has to explicitly add those bind mounts in the same way as
    it was done during initial container creation.
    
    Signed-off-by: Adrian Reber <[email protected]>

/assign kwilczynski

Note

This cherry-pickl brings the following Pull Requests as a dependency:

- Only restore container if all bind mounts are defined.
- Ensure that the "image" attribute has been provided.

Also, ensure that the "image" attribute has been provided. Signed-off-by: Krzysztof Wilczyński <[email protected]>

To avoid the situation where a container that is restored via a registry mounts unexpected host paths into the container, this changes the restore behaviour of CRI-O. Previously all bind mounted paths in the original container which were defined for example like this: volumeMounts: - mountPath: /data name: data-volume volumes: - name: data-volume hostPath: path: /srv/container/data Were automatically mounted in the restored container without and definition necessary. This lead to the situation that the user does not know which path will be mounted if starting a restored container. Now CRI-O will refuse to restore a container if not all bind mounts are defined via the CRI CreateContainer RPC in the CreateContainerRequest message. CRI-O will now return an error that will look something likes this: Error: the container to restore (7f...be) expects following bind mounts defined (/data,/data2) Now the user has to explicitly add those bind mounts in the same way as it was done during initial container creation. Signed-off-by: Adrian Reber <[email protected]>

Signed-off-by: Sascha Grunert <[email protected]>

openshift-ci-robot · 2024-12-10T10:07:20Z

@kwilczynski: This pull request references Jira Issue OCPBUGS-45908, which is invalid:

release note text must be set and not match the template OR release note type must be set to "Release Note Not Required". For more information you can reference the OpenShift Bug Process.
expected Jira Issue OCPBUGS-45908 to depend on a bug in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but no dependents were found

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

This is a manual cherry-pick of commit 429ef7c

commit 429ef7c366406280cfa3daa15812d14b22953a7a
Author: Adrian Reber <[email protected]>
Date:   Mon Sep 30 23:37:13 2024

   Only restore container if all bind mounts are defined
   
   To avoid the situation where a container that is restored via a registry
   mounts unexpected host paths into the container, this changes the
   restore behaviour of CRI-O.
   
   Previously all bind mounted paths in the original container which were
   defined for example like this:
   
       volumeMounts:
       - mountPath: /data
         name: data-volume
     volumes:
     - name: data-volume
       hostPath:
         path: /srv/container/data
   
   Were automatically mounted in the restored container without and
   definition necessary. This lead to the situation that the user does not
   know which path will be mounted if starting a restored container.
   
   Now CRI-O will refuse to restore a container if not all bind mounts are
   defined via the CRI CreateContainer RPC in the CreateContainerRequest
   message.
   
   CRI-O will now return an error that will look something likes this:
   
   Error: the container to restore (7f...be) expects following bind mounts defined (/data,/data2)
   
   Now the user has to explicitly add those bind mounts in the same way as
   it was done during initial container creation.
   
   Signed-off-by: Adrian Reber <[email protected]>

/assign kwilczynski

[!NOTE]
This cherry-pickl brings the following Pull Requests as a dependency:

Check for nil values when importing container definition #8150

Fix container restore lint and unit tests #8786

- Only restore container if all bind mounts are defined.
- Ensure that the "image" attribute has been provided.

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.

kwilczynski · 2024-12-12T10:59:19Z

/cc @cri-o/cri-o-maintainers

kwilczynski · 2025-01-03T18:42:48Z

/retest

kwilczynski · 2025-01-06T11:43:22Z

/retest

kwilczynski · 2025-01-07T23:35:52Z

/close

openshift-ci · 2025-01-07T23:35:56Z

@kwilczynski: Closed this PR.

In response to this:

/close

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.

openshift-ci-robot · 2025-01-07T23:35:58Z

@kwilczynski: This pull request references Jira Issue OCPBUGS-45908. The bug has been updated to no longer refer to the pull request using the external bug tracker.

In response to this:

This is a manual cherry-pick of commit 429ef7c

commit 429ef7c366406280cfa3daa15812d14b22953a7a
Author: Adrian Reber <[email protected]>
Date:   Mon Sep 30 23:37:13 2024

   Only restore container if all bind mounts are defined
   
   To avoid the situation where a container that is restored via a registry
   mounts unexpected host paths into the container, this changes the
   restore behaviour of CRI-O.
   
   Previously all bind mounted paths in the original container which were
   defined for example like this:
   
       volumeMounts:
       - mountPath: /data
         name: data-volume
     volumes:
     - name: data-volume
       hostPath:
         path: /srv/container/data
   
   Were automatically mounted in the restored container without and
   definition necessary. This lead to the situation that the user does not
   know which path will be mounted if starting a restored container.
   
   Now CRI-O will refuse to restore a container if not all bind mounts are
   defined via the CRI CreateContainer RPC in the CreateContainerRequest
   message.
   
   CRI-O will now return an error that will look something likes this:
   
   Error: the container to restore (7f...be) expects following bind mounts defined (/data,/data2)
   
   Now the user has to explicitly add those bind mounts in the same way as
   it was done during initial container creation.
   
   Signed-off-by: Adrian Reber <[email protected]>

/assign kwilczynski

[!NOTE]
This cherry-pickl brings the following Pull Requests as a dependency:

Check for nil values when importing container definition #8150

Fix container restore lint and unit tests #8786

- Only restore container if all bind mounts are defined.
- Ensure that the "image" attribute has been provided.

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.

kwilczynski · 2025-01-07T23:35:58Z

/reopen

openshift-ci · 2025-01-07T23:36:02Z

@kwilczynski: Reopened this PR.

In response to this:

/reopen

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.

openshift-ci-robot · 2025-01-07T23:36:05Z

@kwilczynski: This pull request references Jira Issue OCPBUGS-45908, which is invalid:

expected the bug to be open, but it isn't
expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is Closed (Not a Bug) instead
release note text must be set and not match the template OR release note type must be set to "Release Note Not Required". For more information you can reference the OpenShift Bug Process.
expected Jira Issue OCPBUGS-45908 to depend on a bug in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but no dependents were found

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

This is a manual cherry-pick of commit 429ef7c

commit 429ef7c366406280cfa3daa15812d14b22953a7a
Author: Adrian Reber <[email protected]>
Date:   Mon Sep 30 23:37:13 2024

   Only restore container if all bind mounts are defined
   
   To avoid the situation where a container that is restored via a registry
   mounts unexpected host paths into the container, this changes the
   restore behaviour of CRI-O.
   
   Previously all bind mounted paths in the original container which were
   defined for example like this:
   
       volumeMounts:
       - mountPath: /data
         name: data-volume
     volumes:
     - name: data-volume
       hostPath:
         path: /srv/container/data
   
   Were automatically mounted in the restored container without and
   definition necessary. This lead to the situation that the user does not
   know which path will be mounted if starting a restored container.
   
   Now CRI-O will refuse to restore a container if not all bind mounts are
   defined via the CRI CreateContainer RPC in the CreateContainerRequest
   message.
   
   CRI-O will now return an error that will look something likes this:
   
   Error: the container to restore (7f...be) expects following bind mounts defined (/data,/data2)
   
   Now the user has to explicitly add those bind mounts in the same way as
   it was done during initial container creation.
   
   Signed-off-by: Adrian Reber <[email protected]>

/assign kwilczynski

[!NOTE]
This cherry-pickl brings the following Pull Requests as a dependency:

Check for nil values when importing container definition #8150

Fix container restore lint and unit tests #8786

- Only restore container if all bind mounts are defined.
- Ensure that the "image" attribute has been provided.

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.

openshift-ci · 2025-01-07T23:36:10Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kwilczynski
Once this PR has been reviewed and has the lgtm label, please assign fidencio 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:

OWNERS

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

openshift-ci · 2025-01-07T23:50:07Z

@kwilczynski: 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/e2e-gcp	`12230e1`	link	true	`/test e2e-gcp`
ci/prow/e2e-agnostic	`12230e1`	link	true	`/test e2e-agnostic`
ci/prow/images	`12230e1`	link	true	`/test images`

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.

kwilczynski · 2025-01-15T02:59:04Z

/cc @cri-o/cri-o-maintainers
/cc @haircommander
/cc @sohankunkerkar

haircommander · 2025-01-15T14:35:56Z

personally I think this can be closed. It's out of upstream support and not used downstream

kwilczynski · 2025-01-15T19:27:58Z

personally I think this can be closed. It's out of upstream support and not used downstream

@haircommander makes sense! Thank you!

kwilczynski · 2025-01-15T19:28:06Z

Closing. This backport is no longer needed.

kwilczynski · 2025-01-15T19:28:13Z

/close

openshift-ci · 2025-01-15T19:28:17Z

@kwilczynski: Closed this PR.

In response to this:

/close

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.

openshift-ci-robot · 2025-01-15T19:28:19Z

@kwilczynski: This pull request references Jira Issue OCPBUGS-45908. The bug has been updated to no longer refer to the pull request using the external bug tracker.

In response to this:

This is a manual cherry-pick of commit 429ef7c

commit 429ef7c366406280cfa3daa15812d14b22953a7a
Author: Adrian Reber <[email protected]>
Date:   Mon Sep 30 23:37:13 2024

   Only restore container if all bind mounts are defined
   
   To avoid the situation where a container that is restored via a registry
   mounts unexpected host paths into the container, this changes the
   restore behaviour of CRI-O.
   
   Previously all bind mounted paths in the original container which were
   defined for example like this:
   
       volumeMounts:
       - mountPath: /data
         name: data-volume
     volumes:
     - name: data-volume
       hostPath:
         path: /srv/container/data
   
   Were automatically mounted in the restored container without and
   definition necessary. This lead to the situation that the user does not
   know which path will be mounted if starting a restored container.
   
   Now CRI-O will refuse to restore a container if not all bind mounts are
   defined via the CRI CreateContainer RPC in the CreateContainerRequest
   message.
   
   CRI-O will now return an error that will look something likes this:
   
   Error: the container to restore (7f...be) expects following bind mounts defined (/data,/data2)
   
   Now the user has to explicitly add those bind mounts in the same way as
   it was done during initial container creation.
   
   Signed-off-by: Adrian Reber <[email protected]>

/assign kwilczynski

[!NOTE]
This cherry-pickl brings the following Pull Requests as a dependency:

Check for nil values when importing container definition #8150

Fix container restore lint and unit tests #8786

- Only restore container if all bind mounts are defined.
- Ensure that the "image" attribute has been provided.

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.

Krzysztof Wilczyński and others added 3 commits December 2, 2024 16:13

Check for nil values when importing container definition

4b5a25d

Also, ensure that the "image" attribute has been provided. Signed-off-by: Krzysztof Wilczyński <[email protected]>

Fix container restore lint report

12230e1

Signed-off-by: Sascha Grunert <[email protected]>

kwilczynski requested a review from mrunalp as a code owner December 2, 2024 07:53

openshift-ci bot assigned kwilczynski Dec 2, 2024

openshift-ci bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Dec 2, 2024

openshift-ci bot requested review from QiWang19 and klihub December 2, 2024 07:53

kwilczynski changed the title ~~Only restore container if all bind mounts are defined~~ [release-1.26] Only restore container if all bind mounts are defined Dec 2, 2024

kwilczynski changed the title ~~[release-1.26] Only restore container if all bind mounts are defined~~ [release-1.26] OCPBUGS-45908: Only restore container if all bind mounts are defined Dec 10, 2024

openshift-ci bot requested a review from a team December 12, 2024 10:59

openshift-ci bot closed this Jan 7, 2025

openshift-ci bot reopened this Jan 7, 2025

openshift-ci bot requested review from haircommander and sohankunkerkar January 15, 2025 02:59

openshift-ci bot closed this Jan 15, 2025

kwilczynski deleted the feature/backport-commit-429ef7c36-to-release-1.26 branch January 15, 2025 19:28

[release-1.26] OCPBUGS-45908: Only restore container if all bind mounts are defined #8803

[release-1.26] OCPBUGS-45908: Only restore container if all bind mounts are defined #8803

Uh oh!

Conversation

kwilczynski commented Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Dec 10, 2024

Uh oh!

kwilczynski commented Dec 12, 2024

Uh oh!

kwilczynski commented Jan 3, 2025

Uh oh!

kwilczynski commented Jan 6, 2025

Uh oh!

kwilczynski commented Jan 7, 2025

Uh oh!

openshift-ci bot commented Jan 7, 2025

Uh oh!

openshift-ci-robot commented Jan 7, 2025

Uh oh!

kwilczynski commented Jan 7, 2025

Uh oh!

openshift-ci bot commented Jan 7, 2025

Uh oh!

openshift-ci-robot commented Jan 7, 2025

Uh oh!

openshift-ci bot commented Jan 7, 2025

Uh oh!

openshift-ci bot commented Jan 7, 2025

Uh oh!

kwilczynski commented Jan 15, 2025

Uh oh!

haircommander commented Jan 15, 2025

Uh oh!

kwilczynski commented Jan 15, 2025

Uh oh!

kwilczynski commented Jan 15, 2025

Uh oh!

kwilczynski commented Jan 15, 2025

Uh oh!

openshift-ci bot commented Jan 15, 2025

Uh oh!

openshift-ci-robot commented Jan 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kwilczynski commented Dec 2, 2024 •

edited

Loading