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

Fix bug squid installation on stratum1 #2

Merged
merged 7 commits into from
Sep 5, 2023

Conversation

ahmam
Copy link

@ahmam ahmam commented Sep 5, 2023

Add variable to make choose to configure squid in stratum 1 or not.

Add squid package for default installation stratum 1 on debian
Add variable "cvmfs_strqtum1_squid" to make choose  to install squid in stratum 1 or not
Add variable to make choose  to install squid in stratum 1 or not
@ahmam ahmam added the bug Something isn't working label Sep 5, 2023
@ahmam ahmam self-assigned this Sep 5, 2023
@btravouillon btravouillon changed the title Fix bug squid installtion on stratum1 Fix bug squid installation on stratum1 Sep 5, 2023
@btravouillon
Copy link

Has this bug been reported to upstream project and acknowledged?

Copy link

@milaflq milaflq left a comment

Choose a reason for hiding this comment

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

@ahmam
Add variable cvmfs_strqtum1_squid

Est-ce un typo? (cvmfs_strqtum1_squid)

defaults/main.yml Outdated Show resolved Hide resolved
Copy link

@milaflq milaflq left a comment

Choose a reason for hiding this comment

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

Sinon, le code est bon de mon point de vue.

@ahmam
Copy link
Author

ahmam commented Sep 5, 2023

Has this bug been reported to upstream project and acknowledged?

it is not reported to upstream because it is really optional it is a feature that we need but I will report it in a pull request.

@btravouillon
Copy link

it is not reported to upstream because it is really optional it is a feature that we need but I will report it in a pull request.

Looks like package squid is also added in upstream PR that you mentionned in #3. Looking further, the squid package is installed on the stratum1 without condition for RHEL-like hosts (https://github.com/galaxyproject/ansible-cvmfs/blob/0.2.21/vars/redhat.yml#L19). Thus I believe you can just add the package, you don't have to introduce any new variable that would change the behaviour of the role for RHEL-like hosts.

@ahmam
Copy link
Author

ahmam commented Sep 5, 2023

it is not reported to upstream because it is really optional it is a feature that we need but I will report it in a pull request.

Looks like package squid is also added in upstream PR that you mentionned in #3. Looking further, the squid package is installed on the stratum1 without condition for RHEL-like hosts (https://github.com/galaxyproject/ansible-cvmfs/blob/0.2.21/vars/redhat.yml#L19). Thus I believe you can just add the package, you don't have to introduce any new variable that would change the behaviour of the role for RHEL-like hosts.

I just added the package squid to comply with redhat and remove the changes made by the variable.

Copy link

@btravouillon btravouillon left a comment

Choose a reason for hiding this comment

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

Looks good to me. 👍

@btravouillon btravouillon merged commit 0de78dd into mila Sep 5, 2023
@btravouillon btravouillon deleted the fix-bug-squid-installtion-on-stratum1 branch September 5, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants