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

Skip creating the yum.repos.d structure on debian based systems #3369

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sbernhard
Copy link
Contributor

If you currently run "subscription-manager repos" on a debian/ubuntu system, the directory /etc/yum.repos.d/ and the file /etc/yum.repos.d/redhat.repo with only the header are created. This is confusing some debian/ubuntu users.

This patch does fix this behaviour.

@sbernhard sbernhard force-pushed the skip_creating_repo_structure branch 2 times, most recently from 5b2c8fd to 7ffb3cf Compare January 12, 2024 14:44
@ptoscano
Copy link
Contributor

The changes in init_repo_file_classes() make sense to me.

As for the rest of the changes: it looks to me that the issue that triggers the addition of the option in RepoFileBase is the fact that RepoActionInvoker unconditionally uses YumRepo. Would you please provided a trace of the call stack that leads to the usage of RepoActionInvoker on debian-based systems? I'd rather fix that, than keep using a repo type (YumRepo) that does not belong to the current system.

@sbernhard
Copy link
Contributor Author

sbernhard commented Jan 12, 2024

Sure. Thanks @pinotree for you answer.

If you run subscription-manager repos:

  File "/usr/sbin/subscription-manager", line 33, in <module>
    sys.exit(load_entry_point('subscription-manager==1.29.35', 'console_scripts', 'subscription-manager')())
  File "/usr/lib/python3/dist-packages/subscription_manager/scripts/subscription_manager.py", line 73, in main
    return managercli.ManagerCLI().main()
  File "/usr/lib/python3/dist-packages/subscription_manager/managercli.py", line 92, in main
    ret: Optional[int] = CLI.main(self)
  File "/usr/lib/python3/dist-packages/subscription_manager/cli.py", line 197, in main
    return cmd.main()
  File "/usr/lib/python3/dist-packages/subscription_manager/cli_command/cli.py", line 410, in main
    return_code = self._do_command()
  File "/usr/lib/python3/dist-packages/subscription_manager/cli_command/repos.py", line 141, in _do_command
    repos = rl.get_repos()
  File "/usr/lib/python3/dist-packages/subscription_manager/repolib.py", line 222, in get_repos
    yum_repo_file = YumRepoFile(options = ['skip_creating_repo'])
  File "/usr/lib/python3/dist-packages/rhsm/repofile.py", line 564, in __init__
    RepoFileBase.__init__(self, path, name, options)
  File "/usr/lib/python3/dist-packages/rhsm/repofile.py", line 371, in __init__
    self.create()
  File "/usr/lib/python3/dist-packages/rhsm/repofile.py", line 405, in create
    traceback.print_stack()

@ptoscano
Copy link
Contributor

So what if you replace the usage of YumRepoFile with AptRepoFile in RepoActionInvoker if HAS_DEB822 is defined? In the end, I don't see how using YumRepoFile in a debian-based would be correct, TBH...

@sbernhard sbernhard force-pushed the skip_creating_repo_structure branch 4 times, most recently from 6b36a0d to 2883279 Compare January 18, 2024 13:15
@sbernhard
Copy link
Contributor Author

Any thoughts on this PR @ptoscano ?

@ptoscano
Copy link
Contributor

ptoscano commented Feb 2, 2024

One question I have is: on a Debian system without this patch, what do you get when running subscription-manager repos?

@sbernhard
Copy link
Contributor Author

/etc/yum.repos.d and a file red hat.repo containing the header.

@jtruestedt
Copy link

Just to add - if you have rpm-repos in your Content View and run "subscription-manager repos", you might end up with repositories in redhat.repo causing other issues like breaking the package-profile-upload to katello

@sbernhard
Copy link
Contributor Author

Can you run the tests @ptoscano ?

@sbernhard
Copy link
Contributor Author

@jirihnidek Can you maybe give some feedback / start the tests?

@jirihnidek
Copy link
Contributor

@sbernhard I'm just testing/reviewing this PR.

Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The file /var/lib/rhsm/repo_server_val/redhat.repo is still created, which could be still confusing. I have two also other requests, because you use wrong names of Python modules.

src/subscription_manager/repolib.py Outdated Show resolved Hide resolved
src/subscription_manager/repolib.py Outdated Show resolved Hide resolved
@sbernhard sbernhard force-pushed the skip_creating_repo_structure branch from 2883279 to 9552aaa Compare March 12, 2024 10:54
@sbernhard
Copy link
Contributor Author

Thanks for the PR. The file /var/lib/rhsm/repo_server_val/redhat.repo is still created, which could be still confusing. I have two also other requests, because you use wrong names of Python modules.

Yes, thats true. I thought, this repo_server_val is a "internal storage" which is required for the sub-man and not something a user would ever see.

@jirihnidek
Copy link
Contributor

Thanks for the PR. The file /var/lib/rhsm/repo_server_val/redhat.repo is still created, which could be still confusing. I have two also other requests, because you use wrong names of Python modules.

Yes, thats true. I thought, this repo_server_val is a "internal storage" which is required for the sub-man and not something a user would ever see.

Hmm, the code is wild. The file /var/lib/rhsm/repo_server_val/redhat.repo is still needed as "internal storage", but I would try to rename it to something like /var/lib/rhsm/repo_server_val/rhsm.sources to not look like an alien from Red Hat world. Thoughts?

@sbernhard
Copy link
Contributor Author

Thanks for the PR. The file /var/lib/rhsm/repo_server_val/redhat.repo is still created, which could be still confusing. I have two also other requests, because you use wrong names of Python modules.

Yes, thats true. I thought, this repo_server_val is a "internal storage" which is required for the sub-man and not something a user would ever see.

Hmm, the code is wild. The file /var/lib/rhsm/repo_server_val/redhat.repo is still needed as "internal storage", but I would try to rename it to something like /var/lib/rhsm/repo_server_val/rhsm.sources to not look like an alien from Red Hat world. Thoughts?

Make sense in general. Maybe "a lot of things" need to be adapted because of tests etc. Should we rename this in a separete PR?

@jirihnidek
Copy link
Contributor

Thanks for the PR. The file /var/lib/rhsm/repo_server_val/redhat.repo is still created, which could be still confusing. I have two also other requests, because you use wrong names of Python modules.

Yes, thats true. I thought, this repo_server_val is a "internal storage" which is required for the sub-man and not something a user would ever see.

Hmm, the code is wild. The file /var/lib/rhsm/repo_server_val/redhat.repo is still needed as "internal storage", but I would try to rename it to something like /var/lib/rhsm/repo_server_val/rhsm.sources to not look like an alien from Red Hat world. Thoughts?

Make sense in general. Maybe "a lot of things" need to be adapted because of tests etc. Should we rename this in a separete PR?

You are right. We can do that in another PR.

@sbernhard
Copy link
Contributor Author

Thanks for the PR. The file /var/lib/rhsm/repo_server_val/redhat.repo is still created, which could be still confusing. I have two also other requests, because you use wrong names of Python modules.

Yes, thats true. I thought, this repo_server_val is a "internal storage" which is required for the sub-man and not something a user would ever see.

Hmm, the code is wild. The file /var/lib/rhsm/repo_server_val/redhat.repo is still needed as "internal storage", but I would try to rename it to something like /var/lib/rhsm/repo_server_val/rhsm.sources to not look like an alien from Red Hat world. Thoughts?

Make sense in general. Maybe "a lot of things" need to be adapted because of tests etc. Should we rename this in a separete PR?

You are right. We can do that in another PR.

Is there something missing for this PR?

@sbernhard
Copy link
Contributor Author

@jirihnidek / @ptoscano Any update for this?

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.

4 participants