Skip to content
This repository has been archived by the owner on Jul 25, 2022. It is now read-only.

Add FS name constraint #2047

Merged
merged 45 commits into from
Aug 6, 2020
Merged

Add FS name constraint #2047

merged 45 commits into from
Aug 6, 2020

Conversation

mkpankov
Copy link
Contributor

@mkpankov mkpankov commented Jul 3, 2020

Signed-off-by: Michael Pankov work@michaelpankov.com

Close #1922


This change is Reviewable

@mkpankov mkpankov added this to the IML EX V3 milestone Jul 3, 2020
@mkpankov mkpankov self-assigned this Jul 3, 2020
@mkpankov mkpankov added the bug label Jul 3, 2020
@mkpankov mkpankov marked this pull request as ready for review July 6, 2020 13:23
@mkpankov mkpankov requested review from jgrund and a team July 6, 2020 13:23
utopiabound
utopiabound previously approved these changes Jul 6, 2020
@utopiabound
Copy link
Contributor

what happens if we auto-discover two file-systems with the same name?

@jgrund
Copy link
Member

jgrund commented Jul 6, 2020

what happens if we auto-discover two file-systems with the same name?

It will fail. This patch is an explicit statement of non-support for that scenario. Where as before it was still unsupported but implicit.

Copy link
Member

@jgrund jgrund left a comment

Choose a reason for hiding this comment

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

Have you tested this patch for docker as well? I'm assuming the change in service-config will be picked up for postgres as well.

.travis.yml Show resolved Hide resolved
.travis.yml Show resolved Hide resolved
chroma_core/lib/service_config.py Outdated Show resolved Hide resolved
tests/framework/services/runner.sh.in Show resolved Hide resolved
Copy link
Member

@jgrund jgrund left a comment

Choose a reason for hiding this comment

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

python changes need reformating.

@mkpankov
Copy link
Contributor Author

mkpankov commented Jul 6, 2020

@jgrund I did no local testing with docker, assumed CI tests it. Can do a local run if you think one is needed.

@jgrund
Copy link
Member

jgrund commented Jul 6, 2020

@jgrund I did no local testing with docker, assumed CI tests it. Can do a local run if you think one is needed.

CI tests that nothing is broken, it's not specifically testing that this extension has been activated (though I'd hope it would fail in this case since the migration should then not function).

@jgrund
Copy link
Member

jgrund commented Jul 6, 2020

One thing to consider is this needs to work with upgrades.

I think enabling the extension should happen here:

@jgrund jgrund closed this Jul 6, 2020
@jgrund jgrund reopened this Jul 6, 2020
@jgrund
Copy link
Member

jgrund commented Jul 6, 2020

Scratch that, it needs to be inside of

as that's the only place that's called for both RPM and docker based setups.

Copy link
Member

@jgrund jgrund left a comment

Choose a reason for hiding this comment

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

This needs to work for upgrades as well.

@jgrund jgrund self-requested a review July 6, 2020 17:19
@mkpankov
Copy link
Contributor Author

mkpankov commented Jul 7, 2020

@jgrund moved the code to _syncdb, so that should handle upgrades

Copy link
Member

@jgrund jgrund left a comment

Choose a reason for hiding this comment

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

Service tests in travis are failing

@mkpankov
Copy link
Contributor Author

mkpankov commented Jul 8, 2020

@jgrund all the tests passed, but IML Build & Test failed with some DNS issue. I think it's intermittent.

19:54:08  �[2mJul 07 16:54:02 DEBUG iml_cmd: Running cmd: Command { std: "vagrant" "ssh" "-c" "iml filesystem detect" "adm", kill_on_drop: false }
19:54:18  �[2mJul 07 16:54:18 DEBUG iml_cmd: Child waiting for output: Child { child: ChildDropGuard { inner: Child { pid: 110020 }, kill_on_drop: false }, stdin: Some(ChildStdin { inner: PollEvented { io: Some(Fd { inner: ChildStdin { .. } }) } }), stdout: None, stderr: None }
19:54:18  �[2mJul 07 16:54:18 DEBUG iml_cmd: Child waiting for output: Child { child: ChildDropGuard { inner: Child { pid: 110021 }, kill_on_drop: false }, stdin: Some(ChildStdin { inner: PollEvented { io: Some(Fd { inner: ChildStdin { .. } }) } }), stdout: None, stderr: None }
19:54:18  �[2mJul 07 16:54:18 DEBUG iml_cmd: Child waiting for output: Child { child: ChildDropGuard { inner: Child { pid: 110022 }, kill_on_drop: false }, stdin: Some(ChildStdin { inner: PollEvented { io: Some(Fd { inner: ChildStdin { .. } }) } }), stdout: None, stderr: None }
19:54:18  �[2mJul 07 16:54:18 DEBUG iml_cmd: Child waiting for output: Child { child: ChildDropGuard { inner: Child { pid: 110023 }, kill_on_drop: false }, stdin: Some(ChildStdin { inner: PollEvented { io: Some(Fd { inner: ChildStdin { .. } }) } }), stdout: None, stderr: None }
19:54:18  Warning: Permanently added '10.73.10.21' (ECDSA) to the list of known hosts.
19:54:18  Warning: Permanently added '10.73.10.12' (ECDSA) to the list of known hosts.
19:54:18  Warning: Permanently added '10.73.10.11' (ECDSA) to the list of known hosts.
19:54:18  Warning: Permanently added '10.73.10.22' (ECDSA) to the list of known hosts.
19:54:18   7 Jul 16:54:17 sntp[9361]: Started sntp
19:54:18   7 Jul 16:54:17 sntp[7335]: Started sntp
19:54:18   7 Jul 16:54:17 sntp[9361]: Error looking up : Name or service not known
19:54:18  Unable to resolve hostname(s)
19:54:18   7 Jul 16:54:17 sntp[7335]: Error looking up : Name or service not known
19:54:18  Unable to resolve hostname(s)
19:54:18   7 Jul 16:54:17 sntp[15237]: Started sntp
19:54:18   7 Jul 16:54:17 sntp[14947]: Started sntp
19:54:18   7 Jul 16:54:17 sntp[15237]: Error looking up : Name or service not known
19:54:18  Unable to resolve hostname(s)
19:54:18   7 Jul 16:54:17 sntp[14947]: Error looking up : Name or service not known
19:54:18  Unable to resolve hostname(s)

@jgrund
Copy link
Member

jgrund commented Jul 8, 2020

@mkpankov Looks like the failure was in the docker tests. It appears iml-docker never moved into the active state: https://build.whamcloud.com/job/manager-for-lustre/1141/execution/node/50/log/?consoleFull

@johnsonw
Copy link
Contributor

Looks like it's currently failing with:

07:13:20      adm: Enabling database extensions...
07:13:20      adm: Traceback (most recent call last):
07:13:20      adm:   File "/bin/chroma-config", line 10, in <module>
07:13:20      adm:     load_entry_point('iml-manager==6.1.0', 'console_scripts', 'chroma-config')()
07:13:20      adm:   File "/usr/share/chroma-manager/chroma_core/lib/service_config.py", line 1052, in chroma_config
07:13:20      adm:     errors = service_config.setup(username, password, ntpserver, check_db_space)
07:13:20      adm:   File "/usr/share/chroma-manager/chroma_core/lib/service_config.py", line 792, in setup
07:13:20      adm:     error = self._setup_database(check_db_space)
07:13:20      adm:   File "/usr/share/chroma-manager/chroma_core/lib/service_config.py", line 655, in _setup_database
07:13:20      adm:     self._syncdb()
07:13:20      adm:   File "/usr/share/chroma-manager/chroma_core/lib/service_config.py", line 621, in _syncdb
07:13:20      adm:     settings.DATABASES["default"]["USER"],
07:13:20      adm:   File "/usr/share/chroma-manager/chroma_core/lib/util.py", line 192, in try_shell
07:13:20      adm:     raise CommandError(cmdline, rc, out, err)
07:13:20      adm: chroma_core.lib.util.CommandError: Command failed: ['psql', '-c', 'CREATE EXTENSION IF NOT EXISTS btree_gist;', '-d', 'chroma', '-U', 'chroma']
07:13:20      adm:     return code 1
07:13:20      adm:     stdout: 
07:13:20      adm:     stderr: ERROR:  permission denied to create extension "btree_gist"
07:13:20      adm: HINT:  Must be superuser to create this extension.

Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
Signed-off-by: Michael Pankov <work@michaelpankov.com>
@mkpankov
Copy link
Contributor Author

mkpankov commented Aug 5, 2020

I've pushed the version with the image

@jgrund jgrund merged commit 4275917 into master Aug 6, 2020
@jgrund jgrund deleted the fs-name branch August 6, 2020 13:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add constraint to filesystem name
5 participants