test rootless_storage_path from storage.conf by QiWang19 · Pull Request #5202 · containers/podman

QiWang19 · 2020-02-13T19:29:31Z

Add test for PR containers/storage#529
test rootless_storage_path from strorage.conf. If user configured rootless_storage_path in storage.conf, podman info should suggest the change.

can be merged after #5929

Signed-off-by: Qi Wang qiwan@redhat.com

rhatdan · 2020-02-19T12:38:17Z

Testing for containers/storage#529

rhatdan · 2020-02-19T12:43:09Z

I think the check shouldbe.
Expect(session.OutputToString).To(ContainString(expect))

looks like this is not related. the result <func() string>: 0x1376d40 doesn't contains or equaal to : /tmp/home/some11410dude/some11410dude
any other way to fix?

rh-atomic-bot · 2020-02-19T16:50:39Z

☔ The latest upstream changes (presumably #5258) made this pull request unmergeable. Please resolve the merge conflicts.

rhatdan · 2020-02-21T17:06:37Z

@QiWang19 Whats the scoop on this. Once this passes tests we can merge storage PR.

QiWang19 · 2020-02-21T17:47:28Z

configPath := "/etc/containers/storage.conf"
desPath := filepath.Join(podmanTest.TempDir, "storage.conf")
cmd := exec.Command("sudo", "cp", configPath, desPath)

the cp failed. I want to try the storage.conf under $HOME

rhatdan · 2020-02-21T17:57:06Z

Remove the tmpdir stuff. That must be the issue.

cp /etc/containers/storage.conf /etc/containers/storage.conf.test
And then copy it back.

QiWang19 · 2020-02-28T00:57:05Z

tests error

time="2020-02-27T16:26:22-05:00" level=error msg="'overlay' is not supported over extfs at \"/home/some14791dude/.local/share/containers/storage/overlay\""
[+0883s]       /home/some14791dude/.local/share/containers/storage

should the

[storage]
driver="overlay"
[storage.options]
mount_program="/usr/bin/fuse-overlayfs"

be added to the dummy storage.conf?

edsantiago · 2020-02-28T01:00:00Z

Why not just driver="vfs"? It really shouldn't matter for purposes of this test

QiWang19 · 2020-02-28T01:09:14Z

Why not just driver="vfs"? It really shouldn't matter for purposes of this test

Doesn't work locally. I can try in the test.

QiWang19 · 2020-02-28T01:11:12Z

I'm vendoring a new branch containers/storage#542.
The graphroot is empty in the dummy storage.conf, to use the rootless_storage_path, the variable assignment should be moved out of the if storageOpts.GraphRoot == ""{} else {} block. And there is a path matching issue.

rh-atomic-bot · 2020-03-05T10:40:31Z

☔ The latest upstream changes (presumably #5397) made this pull request unmergeable. Please resolve the merge conflicts.

QiWang19 · 2020-04-01T19:52:51Z

@rhatdan @edsantiago PTAL. Will get this in for test storage.config rootless_storage_path storage.config?

edsantiago

Destroying a user's setup in $HOME, even if only in CI, is an unexpected side effect that could cause much confusion and harm. Please see if there are ways to avoid that.

edsantiago · 2020-04-01T21:29:54Z

Perhaps it might be clearer to say "this test is only meaningful as rootless"

edsantiago · 2020-04-01T21:35:05Z

This makes me uncomfortable. I think these tests run only in CI, and can't be run by developers, but if that ever changes this could be catastrophic for regular users: it would destroy their existing storage.conf.

Did you consider something such as this instead?

preserved_home := getenv("HOME") (add magic fail-safe such that HOME is restored on exit or failure) setenv("HOME", random-tmpdir) ...then create the storage.conf, etc

Lets try that.

rh-atomic-bot · 2020-04-06T19:29:03Z

☔ The latest upstream changes (presumably #5507) made this pull request unmergeable. Please resolve the merge conflicts.

QiWang19 · 2020-04-06T20:26:31Z

I will rework the tests.

test rootless_storage_path from strorage.conf. If user configured rootless_storage_path in storage.conf, podman info should suggest the change. Signed-off-by: Qi Wang <qiwan@redhat.com>

QiWang19 · 2020-04-22T18:32:24Z

@edsantiago @rhatdan PTAL.

edsantiago

LGTM. Thank you for your perseverance.

edsantiago · 2020-04-22T18:47:09Z

+		out, err := cmd.CombinedOutput()
+		fmt.Println(string(out))
+		Expect(err).To(BeNil())
+		Expect(string(out)).To(ContainSubstring(expect))


It'd be better to compare the string exactly, not substring, but I'm OK with this as it is. Just make a note for future work: exact comparisons are better than substring.

rhatdan · 2020-04-22T18:54:04Z

/approve
/lgtm

openshift-ci-robot · 2020-04-22T18:54:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, QiWang19, rhatdan

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [rhatdan]

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

openshift-ci-robot requested review from mrunalp and vrothberg February 13, 2020 19:29

openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 13, 2020

rhatdan changed the title ~~test rootless_storage_path from strorage.conf~~ [WIP] test rootless_storage_path from strorage.conf Feb 13, 2020

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 13, 2020

QiWang19 force-pushed the path-config-storageconf branch 5 times, most recently from 8152352 to 377af92 Compare February 17, 2020 21:45

rhatdan reviewed Feb 19, 2020

View reviewed changes

QiWang19 force-pushed the path-config-storageconf branch from 377af92 to fd2d5aa Compare February 19, 2020 14:55

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 19, 2020

QiWang19 force-pushed the path-config-storageconf branch from fd2d5aa to 85fc562 Compare February 19, 2020 18:20

openshift-ci-robot added size/M and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 19, 2020

QiWang19 force-pushed the path-config-storageconf branch from 85fc562 to 7317015 Compare February 19, 2020 18:25

openshift-ci-robot added size/L and removed size/M labels Feb 19, 2020

rhatdan changed the title ~~[WIP] test rootless_storage_path from strorage.conf~~ [WIP] test rootless_storage_path from storage.conf Feb 19, 2020

QiWang19 force-pushed the path-config-storageconf branch 2 times, most recently from dd4e445 to a3f3e51 Compare February 20, 2020 21:14

QiWang19 force-pushed the path-config-storageconf branch 2 times, most recently from c6775e8 to c9a7905 Compare February 21, 2020 18:30

QiWang19 force-pushed the path-config-storageconf branch from e3d20b9 to b61b953 Compare February 28, 2020 00:55

QiWang19 force-pushed the path-config-storageconf branch 2 times, most recently from b65a0a2 to b51fe82 Compare March 2, 2020 15:03

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 5, 2020

QiWang19 force-pushed the path-config-storageconf branch from b51fe82 to 8627f3e Compare April 1, 2020 16:48

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 1, 2020

QiWang19 changed the title ~~[WIP] test rootless_storage_path from storage.conf~~ test rootless_storage_path from storage.conf Apr 1, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 1, 2020

edsantiago suggested changes Apr 1, 2020

View reviewed changes

QiWang19 force-pushed the path-config-storageconf branch from 8627f3e to 10dfc4f Compare April 21, 2020 19:29

test rootless_storage_path from strorage.conf

d422799

test rootless_storage_path from strorage.conf. If user configured rootless_storage_path in storage.conf, podman info should suggest the change. Signed-off-by: Qi Wang <qiwan@redhat.com>

QiWang19 force-pushed the path-config-storageconf branch from 10dfc4f to d422799 Compare April 22, 2020 18:02

edsantiago approved these changes Apr 22, 2020

View reviewed changes

openshift-ci-robot assigned rhatdan Apr 22, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 22, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 22, 2020

openshift-merge-robot merged commit 9a910ef into containers:master Apr 22, 2020

github-actions Bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 25, 2023

github-actions Bot locked as resolved and limited conversation to collaborators Sep 25, 2023

Conversation

QiWang19 commented Feb 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhatdan commented Feb 19, 2020

Uh oh!

rhatdan Feb 19, 2020

Choose a reason for hiding this comment

Uh oh!

QiWang19 Feb 19, 2020

Choose a reason for hiding this comment

Uh oh!

rh-atomic-bot commented Feb 19, 2020

Uh oh!

rhatdan commented Feb 21, 2020

Uh oh!

QiWang19 commented Feb 21, 2020

Uh oh!

rhatdan commented Feb 21, 2020

Uh oh!

QiWang19 commented Feb 28, 2020

Uh oh!

edsantiago commented Feb 28, 2020

Uh oh!

QiWang19 commented Feb 28, 2020

Uh oh!

QiWang19 commented Feb 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rh-atomic-bot commented Mar 5, 2020

Uh oh!

QiWang19 commented Apr 1, 2020

Uh oh!

edsantiago left a comment

Choose a reason for hiding this comment

Uh oh!

edsantiago Apr 1, 2020

Choose a reason for hiding this comment

Uh oh!

edsantiago Apr 1, 2020

Choose a reason for hiding this comment

Uh oh!

rhatdan Apr 2, 2020

Choose a reason for hiding this comment

Uh oh!

QiWang19 Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

rh-atomic-bot commented Apr 6, 2020

Uh oh!

QiWang19 commented Apr 6, 2020

Uh oh!

QiWang19 commented Apr 22, 2020

Uh oh!

edsantiago left a comment

Choose a reason for hiding this comment

Uh oh!

edsantiago Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

rhatdan commented Apr 22, 2020

Uh oh!

openshift-ci-robot commented Apr 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

QiWang19 commented Feb 13, 2020 •

edited

Loading

QiWang19 commented Feb 28, 2020 •

edited

Loading