Skip to content

Conversation

@rbtr
Copy link
Collaborator

@rbtr rbtr commented Aug 11, 2022

Signed-off-by: Evan Baker rbtr@users.noreply.github.com

Reason for Change: Due to superficial usage of the NC in the IPAM Pool Monitor, it was considered un-started until it received an NNC with an NC. This change fixes that and considers the reconciler and monitor started as soon as an NNC is ready, whether or not the NC is ready also.

Issue Fixed:

Requirements:

Notes:

@rbtr rbtr requested a review from a team as a code owner August 11, 2022 20:09
@rbtr rbtr requested review from thatmattlong and removed request for a team August 11, 2022 20:09
@rbtr rbtr changed the title accept an nnc with no nc as valid and consider monitor and reconciler started Accept NNC with no NC as valid and consider pool monitor and reconciler started Aug 11, 2022
@rbtr rbtr requested a review from neaggarwMS August 11, 2022 20:10
… started

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
Copy link
Collaborator

@thatmattlong thatmattlong left a comment

Choose a reason for hiding this comment

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

Approving assuming the answer to my question is that it doesn't matter if we don't have a scaler yet and it will get updated later.

}
}

scaler := nnc.Status.Scaler
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we don't have NCs we probably don't have a Status at all yet, does this matter? Will the pool monitor start respecting the real scaler when it comes in later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, next time we get an NNC we will copy the scaler out again, and I think this zero-value scaler is safe to use

@rbtr
Copy link
Collaborator Author

rbtr commented Aug 15, 2022

bypassing broken aks-e check

@rbtr rbtr merged commit 8b5e167 into Azure:master Aug 15, 2022
@rbtr rbtr deleted the fix/cns-no-nc branch August 15, 2022 19:59
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.

2 participants