8000 Avoid creating duplicated settings files when automigration is disabled by meaksh · Pull Request #2974 · cobbler/cobbler · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Avoid creating duplicated settings files when automigration is disabled #2974

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

Merged
merged 3 commits into from
Mar 21, 2022

Conversation

meaksh
Copy link
Member
@meaksh meaksh commented Mar 15, 2022

We introduced a PR #2966 to disable setting automigration to be executed by default, this works fine but we just noticed there are still duplicated settings files being created on each restart of cobbler or during instantiation of CobblerAPI object:

suma-test-srv:~ # ls -ld /etc/cobbler/settings*
-rw-r--r-- 1 root   root    40 mar 15 06:55 /etc/cobbler/settings
-rw-r----- 1 root   root  4892 mar 15 01:44 /etc/cobbler/settings_20220315_01-44-38.yaml
-rw-r----- 1 root   root  5389 mar 15 01:45 /etc/cobbler/settings_20220315_01-45-13.yaml
-rw-r----- 1 root   root  5388 mar 15 01:46 /etc/cobbler/settings_20220315_01-46-12.yaml
-rw-r----- 1 root   root  5388 mar 15 03:47 /etc/cobbler/settings_20220315_03-47-21.yaml
-rw-r----- 1 root   root  5388 mar 15 03:48 /etc/cobbler/settings_20220315_03-48-21.yaml
-rw-r----- 1 root   root  5388 mar 15 06:55 /etc/cobbler/settings_20220315_06-55-46.yaml
-rw-r----- 1 root   root  5388 mar 15 06:58 /etc/cobbler/settings_20220315_06-58-10.yaml
-rw-r----- 1 root   root  5388 mar 15 09:03 /etc/cobbler/settings_20220315_09-03-42.yaml
-rw-r----- 1 root   root  5388 mar 15 11:38 /etc/cobbler/settings_20220315_11-38-08.yaml
-rw-r----- 1 root   root  5388 mar 15 11:38 /etc/cobbler/settings_20220315_11-38-45.yaml
-rw-r----- 1 root   root  5388 mar 15 11:40 /etc/cobbler/settings_20220315_11-40-07.yaml
-rw-r----- 1 root   root  5388 mar 15 11:40 /etc/cobbler/settings_20220315_11-40-25.yaml
-rw-r----- 1 root   root  5388 mar 15 11:42 /etc/cobbler/settings_20220315_11-42-10.yaml
-rw-r----- 1 root   root  5388 mar 15 12:09 /etc/cobbler/settings_20220315_12-09-19.yaml
-rw-r----- 1 wwwrun www   5388 mar 15 12:09 /etc/cobbler/settings_20220315_12-09-40.yaml<
8000
/span>
-rw-r----- 1 root   root  5388 mar 15 12:10 /etc/cobbler/settings_20220315_12-10-21.yaml
-rw-r----- 1 wwwrun www   5388 mar 15 12:10 /etc/cobbler/settings_20220315_12-10-27.yaml
-rw-r----- 1 wwwrun www   5388 mar 15 12:11 /etc/cobbler/settings_20220315_12-11-04.yaml
drwxr-x--- 2 root   www    119 mar 15 01:37 /etc/cobbler/settings.d
-rw-r----- 1 root   www   5388 mar 15 12:11 /etc/cobbler/settings.yaml
-rw-r--r-- 1 root   root 22573 mar 15 01:44 /etc/cobbler/settings.yaml.backup
-rw-r----- 1 root   root  5389 mar 15 01:45 /etc/cobbler/settings.yaml.bak

This PR simply fixed this issue by preventing we save a copy of the settings when automigration was not performed.

@SchoolGuy SchoolGuy added this to the v3.4.0 milestone Mar 15, 2022
@SchoolGuy SchoolGuy requested a review from a team March 15, 2022 11:23
@codecov
Copy link
codecov bot commented Mar 15, 2022

Codecov Report

Merging #2974 (d6c38d8) into main (7324b19) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head d6c38d8 differs from pull request most recent head cea0f6a. Consider uploading reports for the commit cea0f6a to get more accurate results

@@           Coverage Diff           @@
##             main    #2974   +/-   ##
=======================================
  Coverage   62.02%   62.03%           
=======================================
  Files         102      102           
  Lines       14427    14428    +1     
=======================================
+ Hits         8949     8950    +1     
  Misses       5478     5478           
Impacted Files Coverage Δ
cobbler/api.py 60.97% <100.00%> (+0.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7324b19...cea0f6a. Read the comment docs.

Copy link
Member
@SchoolGuy SchoolGuy left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix. Sorry that I have overlooked this.

@SchoolGuy
Copy link
Member

@meaksh Could you squash the black commit into your commit where you introduced the changes please?

@meaksh meaksh force-pushed the main-fix-settings-files-regeneration branch from d6c38d8 to cea0f6a Compare March 21, 2022 11:03
@meaksh
Copy link
Member Author
meaksh commented Mar 21, 2022

@SchoolGuy done 👍

@SchoolGuy SchoolGuy merged commit 4706f96 into main Mar 21, 2022
@SchoolGuy SchoolGuy deleted the main-fix-settings-files-regeneration branch March 21, 2022 12:54
SchoolGuy added a commit that referenced this pull request Apr 19, 2022
This backports:

- #2972
- #2973
- #2974
- #2976
- #2985
- #2986
- #2989
- #2990
- #2991
- #2996<
8000
/a>
- #3008
- #3009
- #3017

The exact content of the patches can please be taken from the Pull Requests on
GitHub.
SchoolGuy added a commit that referenced this pull request Apr 20, 2022
This backports:

- #2973
- #2974
- #2976
- #2985
- #2986
- #2989
- #2990
- #2991
- #2996
- #3008
- #3009
- #3017

The exact content of the patches can please be taken from the Pull Requests on
GitHub.
SchoolGuy added a commit that referenced this pull request Apr 20, 2022
This backports:

- #2973
- #2974
- #2976
- #2985
- #2986
- #2989
- #2990
- #2991
- #2996
- #3008
- #3009
- #3017

The exact content of the patches can please be taken from the Pull Requests on
GitHub.
SchoolGuy added a commit that referenced this pull request Apr 20, 2022
This backports:

- #2973
- #2974
- #2976
- #2985
- #2986
- #2989
- #2990
- #2991
- #2996
- #3008
- #3009
- #3017

The exact content of the patches can please be taken from the Pull Requests on
GitHub.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants
0