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

Jump to bottom

[pulpcore] add plugin for pulp-3 #2512

Closed
wants to merge 1 commit into from
Closed

[pulpcore] add plugin for pulp-3 #2512

wants to merge 1 commit into from

Conversation

pmoravec
Copy link
Contributor

Pulp-3 / pulpcore as a revolution from pulp-2 needs a separate
plugin, since both plugins have nothing in common and there might
be deployments where is active both pulp-2 and pulp-3.

Resolves: #2278

Signed-off-by: Pavel Moravec pmoravec@redhat.com

Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname email@example.com?
  • If this commit closes an existing issue, is the line Closes: #ISSUENUMBER included in an independent line?
  • If this commit resolves an existing pull request, is the line Resolves: #PRNUMBER included in an independent line?

@pmoravec
Copy link
Contributor Author

pmoravec commented Apr 27, 2021 •
edited

This is so far a WIP version of the plugin, that needs some review from pulp devels and some more testing.

@goosemania , could you please (ask for a) review of the plugin?

For testing on a sos-3.9 version, it is sufficient to:

  • copy the file to sos/plugins/pulpcore.py
  • sed 's/sos.report.plugins/sos.plugins/1'
  • sed 's/IndependentPlugin/RedHatPlugin, DebianPlugin, UbuntuPlugin/g'

Points to consider:

  • where is PULP_CONCURRENCY / number of pulp workers collected?
  • is it worth to collect some redis config (we have a dedicated redist plugin, though..)
  • is it worth collecting pulpcore-manager output? (I havent seen anything useful there)
  • dynaconf list has to be called from pulp user (that is bit tricky for sos, I think)? It works well under root as well
  • in parallel to all tasks (from past 7 days), does it make sense to collect all running tasks with their reserved resources? (duplicate data that one would need to extract from the "all tasks" ones)

cc @jjansky1

TurboTurtle
TurboTurtle reviewed Apr 27, 2021
View reviewed changes
Copy link
Member

@TurboTurtle TurboTurtle left a comment

Choose a reason for hiding this comment

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

Initial comments below, very minor points. Only other question I have is why pulpcore instead of pulp3? I don't necessarily mind using the former, I'm just curious as to the reasoning.

sos/report/plugins/pulpcore.py Outdated
@@ -0,0 +1,121 @@
# Copyright (C) 2018 Red Hat, Inc., Jake Hunsaker <jhunsake@redhat.com>
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. ;-)

sos/report/plugins/pulpcore.py Show resolved Hide resolved
sos/report/plugins/pulpcore.py Outdated
Comment on lines 81 to 86
_hname = self.exec_cmd('hostname')['output']
_hname = _hname.strip()
self.add_cmd_output("rq info -u redis://localhost:6379/8",
env={"LC_ALL": "en_US.UTF-8"},
suggest_filename="rq_info")
self.add_cmd_output("curl -s https://%s/pulp/api/v3/status/" % _hname,
suggest_filename="pulp_status")
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason we use localhost in one place and the actual hostname in another? Can both not use localhost?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes and no: curl-ing localhost fails on SSL certificate/hostname check. But --insecure option skips that check, so I will use it.

Technically, the very proper solution is to use CONTENT_HOST = "my.fqdn" from /etc/pulp/settings.py but 1) we would have to have a failover value and 2) the hostname will very rarely differ from the local host. So till --insecure works, I am in favour of localhost, for the sake of simplicity.

@pmoravec
Copy link
Contributor Author

Initial comments below, very minor points. Only other question I have is why pulpcore instead of pulp3? I don't necessarily mind using the former, I'm just curious as to the reasoning.

No strong argument or opinion here. pulp-3 is the "branding" name, pulpcore is the classes name. Maybe pulp3 is more informative to users of sosreport, to stress it is the "replacement" of pulp plugin / pulp-2?

IMHO short description should have both terms, plugin name: no much preference (now I tend to pulp3 bit). If somebody agrees with pulp3, let comment it and I will rename the plugin.

@pmoravec
[pulpcore] add plugin for pulp-3
6563091 
Pulp-3 / pulpcore as a revolution from pulp-2 needs a separate
plugin, since both plugins have nothing in common and there might
be deployments where is active both pulp-2 and pulp-3.

Resolves: sosreport#2278

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>
@pmoravec pmoravec force-pushed the sos-pmoravec-pulpcore-plugin branch from fc0d4d9 to 6563091 Compare April 28, 2021 07:52
@evgeni
Copy link
Contributor

evgeni commented May 3, 2021

We've been using "pulpcore" in various places (Puppet module, Repository name, etc), as we didn't want to hardcode the 3 to differentiate from pulp(2) which is usually referred to as "Pulp".

@TurboTurtle
Copy link
Member

Merged via 808d9f3

@TurboTurtle TurboTurtle closed this May 3, 2021
pmoravec added a commit to pmoravec/sos that referenced this pull request May 4, 2021
@pmoravec
[pulpcore] add plugin for pulp-3
ff681f3 
Backport sosreport#2512 to add pulpcore plugin also to legacy-3.9 branch.

Related: sosreport#2512
Resolves: sosreport#2520

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>
@pmoravec pmoravec mentioned this pull request May 4, 2021
Closed
6 tasks
@ekohl
Copy link
Contributor

ekohl commented May 4, 2021

I missed this PR. Is there a good way to stay on top of these (Foreman, Pulp and the subsystems it depends on)?

@pmoravec
Copy link
Contributor Author

pmoravec commented May 4, 2021

I missed this PR. Is there a good way to stay on top of these (Foreman, Pulp and the subsystems it depends on)?

Yeah I thought we get some feedback before merging the PR, but never mind, we can iterate / fine-tune the plugin further, for sure. Let open an issue if you spot something (and we can make the filename a variable as well, as a side-fix).

@ekohl
Copy link
Contributor

ekohl commented May 5, 2021

Next time, feel free to ping me in a PR.

TurboTurtle pushed a commit that referenced this pull request May 5, 2021
@pmoravec @TurboTurtle
[pulpcore] add plugin for pulp-3
36c678f 
Backport #2512 to add pulpcore plugin also to legacy-3.9 branch.

Related: #2512
Resolves: #2520

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>
Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TurboTurtle TurboTurtle TurboTurtle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[pulp] Update the plugin for pulp-3 properly
4 participants
@pmoravec @evgeni @TurboTurtle @ekohl