Closed giladravid16 closed 3 weeks ago
Hi @giladravid16. Thanks for your PR.
I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test
on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test
label.
I understand the commands that are listed here.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set.
MGMT-16212 talks about generic requirements, but the change seems specific to the MCE operator. @giladravid16 could you please clarify what are the intentions of this PR by updating description and title accordingly?
MGMT-16212 is specifically for the assisted-installer component and the MCE operator handles the logic of validating the hosts before installation
MGMT-16212 is specifically for the assisted-installer component and the MCE operator handles the logic of validating the hosts before installation
What do you mean by "specifically for the assisted-installer component"? Is this PR intention to be flexible with requirements under all circumstances, or only when deploying MCE? Please do change the description to of this PR to describe what are we trying to do exactly here
The MCE operator should handle logic of additional requirements due to the MCE operator deployment. For generic/total requirements I think this is the place that handles them
I missed that the code change was in the MCE requirements, that was not the intention. The ticket wants us to address the final memory requirement which combines all operators and the base requirement, not just the requirement for the MCE operator.
@giladravid16 if this requires clarification feel free to reach out with questions
@giladravid16 Nice work, few things:
MGMT-16212: <Your commit message here>
. Also remove the 'merge' commit, looks like it's not needed.I'll write here what I understood the flow is and what should be done.
There are 4 function that are used to calculate whether the host has enough memory and they are divided into 2 groups:
But all 4 functions use the versioned host requirements so it is best to change what is returned from that to already take the tolerance into account. So the function that should be updated is this.
Is this correct?
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 68.62%. Comparing base (
1f7cce3
) to head (b22f648
). Report is 12 commits behind head on master.
/retest-required
/ok-to-test
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.
/retest
/retest
/retest
Please make sure to include your summary inside the commit message as well, so that the motivation is accessible through git blame
, as described in the checklist:
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.
@giladravid16: This pull request references MGMT-16212 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: eliorerz, giladravid16
The full list of commands accepted by this bot can be found here.
The pull request process is described here
@giladravid16: all tests passed!
Full PR test history. Your PR dashboard.
[ART PR BUILD NOTIFIER]
Distgit: ose-agent-installer-api-server This PR has been included in build ose-agent-installer-api-server-container-v4.18.0-202408181243.p0.g3220e6f.assembly.stream.el9. All builds following this will include this PR.
We want to allow hosts to have slightly less memory than in the official documentation because some setups (like VMs) might give slightly less than the required amount. This PR changes the host validator to tolerate a slightly lower memory amount.
Apart from automatic tests this was also tested in the following way:
List all the issues related to this PR
What environments does this code impact?
How was this code tested?
Checklist
docs
, README, etc)Reviewers Checklist