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

Issue-562 | Added code changes to update pod scheduling error message… #566

Merged
merged 3 commits into from
Mar 22, 2022

Conversation

Rahul1804
Copy link
Collaborator

@Rahul1804 Rahul1804 commented Mar 14, 2022

Details about code changes

#562

Testing

Created 5 member ubuntu cluster

Screenshot 2022-03-18 at 12 12 52 AM

Couple of pods' scheduling failed because of less resource availability in the k8s cluster.

Screenshot 2022-03-17 at 11 56 04 PM

Then untainted the master node so that some of the pods can be scheduled on master.

Screenshot 2022-03-17 at 11 57 16 PM

Change in kd node id 5 status - Configured And No Scheduling Error Message

Screenshot 2022-03-18 at 12 00 45 AM

@Rahul1804 Rahul1804 requested a review from joel-bluedata March 14, 2022 05:46
@joel-bluedata
Copy link
Member

Plz add some details here about what you tested and an example of what the relevant status fields look like in one of the test cases.

@@ -619,6 +623,9 @@ func updateStateRollup(
// Count missing container as waiting, at this point.
if memberStatus.StateDetail.LastKnownContainerState == containerMissing {
cr.Status.MemberStateRollup.MembersWaiting = true
if memberStatus.StateDetail.SchedulingErrorMessage != nil {
cr.Status.MemberStateRollup.MembersNotScheduled = true
Copy link
Contributor

Choose a reason for hiding this comment

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

In the block of stuff at the beginning of this function we must also set MembersNotScheduled to false initially.

@@ -0,0 +1,37 @@
// Copyright 2019 Hewlett Packard Enterprise Development LP
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright 2022!

@neogeographica
Copy link
Contributor

Just a couple of minor fixes noted above, then this will be ready to merge.

@neogeographica
Copy link
Contributor

Sorry I'm logged into my personal GitHub account rather than corporate. Lemme switch...

@joel-bluedata
Copy link
Member

OK this should make it more official. :-) Just a couple of minor fixes noted above, then this will be ready to merge.

@joel-bluedata joel-bluedata merged commit 6a96ec4 into bluek8s:master Mar 22, 2022
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