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 getCurrentSpareInstanceCount - check busy nodes in given fleet, not whole Jenkins instance #343

Merged
merged 5 commits into from
Aug 18, 2022

Conversation

h-okon
Copy link

@h-okon h-okon commented Aug 10, 2022

This fixes #342.
getCurrentSpareInstanceCount checks in whole Jenkins computers and does not account that we want to check only in given fleet.
I added a filter to filter worker nodes only from given fleet that is being checked. If we are not checking the computer from given fleet - we continue, we don't want to count the computer as busy computer in given fleet, because it is not a part of given fleet.

@h-okon
Copy link
Author

h-okon commented Aug 16, 2022

@haugenj @imuqtadir Is there a possibility to kindly ask for a review please?

@brycahta
Copy link

@h-okon , thanks for both opening the issue and following up with a fix. I'll review this today.

@haugenj @imuqtadir Is there a possibility to kindly ask for a review please?

Unfortunately (for us), those guys are no longer with the team

Copy link

@brycahta brycahta left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Could you just add a comment to the added code with your explanation?

@h-okon
Copy link
Author

h-okon commented Aug 16, 2022

I have added a comment explaining the code

Copy link

@brycahta brycahta left a comment

Choose a reason for hiding this comment

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

/lgtm

Apologies for any delays getting this merged, we are in the process of transferring some permissions.

@mwos-sl
Copy link

mwos-sl commented Aug 17, 2022

Can we have a new release after this one is merged? I think we observed the same and would love to apply the fix asap.

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.

After setting up minimum spare instances - instances are not deprovisioned if demand is lower than requested
3 participants