Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BZ#2188239 - better detect already migrated PostgreSQL data #1071

Closed
wants to merge 1 commit into from

Conversation

evgeni
Copy link
Member

@evgeni evgeni commented Apr 21, 2023

Users can (and in some cases have to) manually move the PostgreSQL data to the new location. In the case when they would completely delete the old location, the facts code would previously error out:

Traceback (most recent call last):
  File "/usr/lib64/python2.7/multiprocessing/process.py", line 258, in _bootstrap
    self.run()
  File "/usr/lib64/python2.7/multiprocessing/process.py", line 114, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/lib/python2.7/site-packages/leapp/repository/actor_definition.py", line 72, in _do_run
    actor_instance.run(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/leapp/actors/init.py", line 289, in run
    self.process(*args)
  File "/usr/share/leapp-repository/repositories/system_upgrade/el7toel8/actors/satellite_upgrade_facts/actor.py", line 96, in process
    scl_psql_stat = os.stat(POSTGRESQL_SCL_DATA_PATH)
OSError: [Errno 2] No such file or directory: '/var/opt/rh/rh-postgresql12/lib/pgsql/data/'

However, if the user already has migrated the data, we don't have to do it in the actor and thus can skip all that detection code.

Also adjust the report to say the data looks migrated (but still needs a reindex).

@github-actions
Copy link

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergeable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

  • review please to notify leapp developers of review request
  • /packit copr-build to submit a public copr build using packit

To launch regression testing public members of oamg organization can leave the following comment:

  • /rerun to schedule basic regression tests using this pr build and leapp*master* as artifacts
  • /rerun 42 to schedule basic regression tests using this pr build and leapp*PR42* as artifacts
  • /rerun-sst to schedule sst tests using this pr build and leapp*master* as artifacts
  • /rerun-sst 42 to schedule sst tests using this pr build and leapp*PR42* as artifacts

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra.

@evgeni
Copy link
Member Author

evgeni commented Apr 21, 2023

/rerun

@github-actions
Copy link

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/5812223

@github-actions
Copy link

Testing Farm request for RHEL-8.6-rhui/5812223 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-rhui/5812223 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-8.6.0-Nightly/5812223 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-8.8.0-Nightly/5812223 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-ZStream/5812223 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@github-actions
Copy link

Testing Farm request for RHEL-7.9-ZStream/5812223 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

@evgeni
Copy link
Member Author

evgeni commented Apr 24, 2023

Failed loading plugin "copr": module 'dnf' has no attribute 'cli'

this doesn't look like a related error?

@fernflower
Copy link
Member

@evgeni It is unrelated, yeah, sorry about that. It has emerged recently and we are actively investigating it.
BTW you don't need to leave a '/rerun' comment anymore, now packit will schedule tests for you. I guess I need to update the PR bot message :)

@evgeni evgeni marked this pull request as ready for review October 22, 2023 11:19
@pirat89 pirat89 requested a review from fernflower November 3, 2023 15:21
@pirat89
Copy link
Member

pirat89 commented Nov 13, 2023

@lpramuk Hi Lukas, can you (or someone) test it before the merge and confirm it works as expected?

@@ -48,7 +54,7 @@ def satellite_upgrade_check(facts):
so that the contents of {} are available in /var/lib/pgsql/data.
""".format(scl_psql_path, storage_message, scl_psql_path)

summary = "{}\n{}".format(textwrap.dedent(migration_msg).strip(), reindex_msg)
summary = "{}\n{}\n{}".format(intro_msg, textwrap.dedent(migration_msg).strip(), reindex_msg)
Copy link
Member

Choose a reason for hiding this comment

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

@evgeni hmm...not sure how the msg will look like in various web UIs. From our experience, new-lines does not look always so good as in the .txt file. But it can be kept as it is, just letting you know that we already changed some texts in the past (rarely..) as they looked very weird. e.g. autowrapping plays a role here.

Copy link
Member

@pirat89 pirat89 left a comment

Choose a reason for hiding this comment

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

lgtm. waiting for @lpramuk for the feedback about the testing for now.

@pirat89
Copy link
Member

pirat89 commented Nov 13, 2023

/packit build

Users can (and in some cases have to) manually move the PostgreSQL data
to the new location. In the case when they would completely delete the
old location, the facts code would previously error out:

    Traceback (most recent call last):
      File "/usr/lib64/python2.7/multiprocessing/process.py", line 258, in _bootstrap
        self.run()
      File "/usr/lib64/python2.7/multiprocessing/process.py", line 114, in run
        self._target(*self._args, **self._kwargs)
      File "/usr/lib/python2.7/site-packages/leapp/repository/actor_definition.py", line 72, in _do_run
        actor_instance.run(*args, **kwargs)
      File "/usr/lib/python2.7/site-packages/leapp/actors/init.py", line 289, in run
        self.process(*args)
      File "/usr/share/leapp-repository/repositories/system_upgrade/el7toel8/actors/satellite_upgrade_facts/actor.py", line 96, in process
        scl_psql_stat = os.stat(POSTGRESQL_SCL_DATA_PATH)
    OSError: [Errno 2] No such file or directory: '/var/opt/rh/rh-postgresql12/lib/pgsql/data/'

However, if the user already has migrated the data, we don't have to do
it in the actor and thus can skip all that detection code.

Also adjust the report to say the data looks migrated (but still needs a
reindex).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants