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

Improve join robustness #35

Closed
wants to merge 0 commits into from

Conversation

martencassel
Copy link
Contributor

@martencassel martencassel commented Oct 13, 2024

This PR will attempt to improve join robustness and handle the cases of

  1. We can authentication errors, we perform retries for a number of times until giving up.
  2. We handle existing accounts, and optionally from the config, skip touching these accounts.

Handling

#22
#27
#20

@martencassel martencassel marked this pull request as draft October 13, 2024 22:04
end

def retry_authentication_error(enroll)
MAX_RETRIES.times do |i|
Copy link

@dsnpl dsnpl Dec 24, 2024

Choose a reason for hiding this comment

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

This looks really good to me and much better than (dsnpl@a18ce9c88e321ad6399b884e87388fcb5ce57a74)[my attempt] -

I found that with multiple DCs requests get sequentially round-robined and that the set password request after the create create object request goes to a different DC that hasnt had the new object synced to it. Retrying for up to about 10 seconds sorts this out for me.

@martencassel martencassel marked this pull request as ready for review December 25, 2024 11:24
@martencassel
Copy link
Contributor Author

Ping @ekohl

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

This PR does a few things. Would you mind separating it into smaller PR? Preferably start with the CI updates

Copy link
Member

Choose a reason for hiding this comment

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

For CI we've introduced a common workflow but not app plugins have adopted it. I'd prefer to do that. https://github.com/theforeman/actions?tab=readme-ov-file#smart-proxy-plugin-test has an example

Dockerfile Outdated
@@ -0,0 +1,6 @@
FROM ruby:2.7
Copy link
Member

Choose a reason for hiding this comment

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

We are in the process of dropping Ruby 2.7. If we introduce a container I'd prefer it to use Ruby 3

@martencassel martencassel deleted the join_robustness branch February 20, 2025 20:15
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