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

Update client repository to satellite-6-client-2 #3247

Closed

Conversation

AkshayGadhaveRH
Copy link
Contributor

  • The newer versions support puppet-agent 8.
  • The client repo is versionless and older versions use puppet-agent 7.
  • To work around this, satellite-6-client-2 is created for the later versions with puppet-agent 8.
  • Removed the snippet linked in Installing and configuring Puppet agent manually as this snippet is used in multiple modules.

JIRA: https://issues.redhat.com/browse/SAT-27092

What changes are you introducing?

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

Checklists

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

  • Foreman 3.12/Katello 4.14 (Satellite 6.16)
  • Foreman 3.11/Katello 4.13
  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • Foreman 3.6/Katello 4.8
  • Foreman 3.5/Katello 4.7 (Satellite 6.13; orcharhino 6.6/6.7)
  • We do not accept PRs for Foreman older than 3.5.

Copy link

github-actions bot commented Aug 30, 2024

@ekohl ekohl added the Waiting on contributor Requires an action from the author label Aug 30, 2024
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Sep 2, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this change. Instead, you can either adjust the attribute in guides/common/modules/snip_prerequisite-project-client-repository-enabled.adoc, or, if necessary, change the attribute value in https://github.com/theforeman/foreman-documentation/blob/master/guides/common/attributes-satellite.adoc#L164.

Copy link
Contributor

Choose a reason for hiding this comment

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

@AkshayGadhaveRH Maybe reverting is not an option if you explicitely only want version 2 in this procedure. But I still wonder if it makes sense to reference different client versions in different procedures.

guides/common/attributes-satellite.adoc Show resolved Hide resolved
@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Needs re-review labels Sep 2, 2024
Copy link
Contributor

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

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

sorry for the confusing comments!

Copy link
Contributor

Choose a reason for hiding this comment

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

@AkshayGadhaveRH Maybe reverting is not an option if you explicitely only want version 2 in this procedure. But I still wonder if it makes sense to reference different client versions in different procedures.

guides/common/attributes-satellite.adoc Show resolved Hide resolved
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Sep 2, 2024
@Lennonka
Copy link
Contributor

Lennonka commented Sep 2, 2024

Aren't we supposed to support both repos at this time? So instead of replacing it, we should add it?

@ekohl
Copy link
Member

ekohl commented Sep 2, 2024

Good question. I think we want users to use the latest version. I think there's one edge case: if you have n-1 Capsules then they may run Puppetserver 7. You're only supposed to use agents that are at most as new as the server, so having Puppet 8 on clients with a Capsule 6.15 should be unsupported.

Not all Capsules will run Puppetserver, but out of simplicity I'd say that you need to have upgraded the server (Satellite or Capsule, depending on how the host is configured) to 6.16 to use satellite-6-client-2. Otherwise you need to stick to the old version.

It's possible to have a mixed environment where some hosts are connected to Satellite 6.16 using satellite-6-client-2 while other hosts are connected to a Capsule 6.15 using the old version. If the user is careful I don't see how that would be a problem, but generally I'd say users should upgrade their entire deployment (Satellite and all Capsules) before migrating to satellite-6-client-2.

Should that be covered in the upgrading guide as well? Probably also a release note, though I know we don't capture Satellite release notes here.

@asteflova
Copy link
Member

Good question. [...] I think there's one edge case: [...] If the user is careful I don't see how that would be a problem, but generally I'd say users should [...]

From our d/s perspective, we should not try to document every possible scenario. We should stick to the most user-friendly (and, I suppose, least error-prone) solutions. Without understanding all the details of this particular situation, I just wanted to chime in with my 2 cents: If there is a recommended scenario among many other different scenarios that could go badly if users aren't careful, we should document the recommended one to steer users towards those user stories that we actually want to support.

TL;DR: We shouldn't document everything that's possible. We should document the things that we want users to do.

@ekohl
Copy link
Member

ekohl commented Sep 2, 2024

TL;DR: We shouldn't document everything that's possible. We should document the things that we want users to do.

I agree, which is why I emphasized on the simple solutions. But we must think about the edge cases because customers will come up with them. If we don't clearly document that users of n-1 first need to upgrade, it's likely that some customer will attempt to do so.

More concrete, we have this line in our Satellite upgrade guide:

Note that you can upgrade Capsules separately from Satellite.

And this line:

Note that Satellite Server upgraded from 6.15 to 6.16 can use Capsule Servers still at 6.15.

That will now have an asterisk that there are some limitations if you do.

@Lennonka
Copy link
Contributor

Lennonka commented Sep 4, 2024

I rather meant that in Sat 6.16 we support Puppet agent 7 in the Client 1 repo alongside Puppet agent 8 in the Client 2 repo. So we need to add a new attribute instead of replacing the existing value.

@AkshayGadhaveRH
Copy link
Contributor Author

@Lennonka I'll retain the old attribute as well.

@AkshayGadhaveRH
Copy link
Contributor Author

@evgeni @ehelms where should the new repos replace the old ones?

@Lennonka
Copy link
Contributor

Lennonka commented Sep 23, 2024

where should the new repos replace the old ones?

We need to update the following:

@Lennonka Lennonka added the Waiting on contributor Requires an action from the author label Sep 23, 2024
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Oct 3, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

The build is failing because you have renamed this snippet, but it has other occurences and now it cannot be found there. But it's unnecessary to rename this snippet because it is not limited to Puppet. Please, undo the renaming.

@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Needs re-review labels Oct 7, 2024
@Lennonka
Copy link
Contributor

Lennonka commented Oct 8, 2024

FYI With recent updates from the RH engineering, the Client 2 repo will not be provided in 6.16.0 GA. It will be provided later. So we need to monitor it and merge this PR later.

Akshay Gadhave added 8 commits October 9, 2024 11:40
- The newer versions support puppet-agent 8.
- The client repo is versionless and older versions use puppet-agent 7.
- To work around this, `satellite-6-client-2` is created for the later versions with puppet-agent 8.
- Removed the snippet linked in `Installing and configuring Puppet agent manually` as this snippet is used in multiple modules.

JIRA: https://issues.redhat.com/browse/SAT-27092
Introduced the project-client-2-name attribute for Satellite and set it to satellite-6-client-2.
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Oct 9, 2024
@AkshayGadhaveRH
Copy link
Contributor Author

Hey @Lennonka fixed it. I had preserved the old snippet as well, but I think the build was failing because the project-client-2-name attribute was not defined for non-Satellite builds. Added these to the attributes and it seems to build fine now.

@Lennonka
Copy link
Contributor

Lennonka commented Oct 9, 2024

@AkshayGadhaveRH That's good spotting, but I would prefer that we just edit the original snippet instead of adding a new one. It isn't necessary to add the new one because the old one can be reused with edits.

@Lennonka Lennonka added Waiting on contributor Requires an action from the author and removed Needs re-review labels Oct 9, 2024
@AkshayGadhaveRH
Copy link
Contributor Author

@Lennonka would it be better to manipulate the attribute for puppet related modules so that it renders to Client-2 in that case?

@Lennonka
Copy link
Contributor

Lennonka commented Oct 10, 2024

@AkshayGadhaveRH I think we should clarify first, whether we want to tell users that they can use either Client 1 or Client 2 repo, or we tell them only about Client 2, in 6.16.

@ehelms Can you please clarify?

@ehelms
Copy link
Member

ehelms commented Oct 11, 2024

@AkshayGadhaveRH I think we should clarify first, whether we want to tell users that they can use either Client 1 or Client 2 repo, or we tell them only about Client 2, in 6.16.

@ehelms Can you please clarify?

There is no client 2 right now, this change is not needed for the moment. So we can just close it till we are ready.

@AkshayGadhaveRH
Copy link
Contributor Author

Closing this as the changes have only been made to the puppeterver and the installer making these doc changes unnecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Waiting on contributor Requires an action from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants