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

Add a state in cloud account config #286

Merged
merged 2 commits into from
Aug 14, 2023
Merged

Add a state in cloud account config #286

merged 2 commits into from
Aug 14, 2023

Conversation

Anandkumar26
Copy link
Contributor

The cloud account config contains the service configs and cloud api clients. Whenever cloud account config update fails, set the cloud account config state to invalid.

Any consumer of cloud account config which makes the cloud API calls should check the cloud account config state and proceed only if it's in a valid state.

@@ -202,7 +205,9 @@ func (c *cloudCommon) DoInventoryPoll(accountNamespacedName *types.NamespacedNam
}
accCfg.LockMutex()
defer accCfg.UnlockMutex()

if !accCfg.GetAccountConfigState() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you just update GetCloudAccountByName routine to return 'not found' or 'not valid' error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the function to return, account config, found and an error.

@reachjainrahul
Copy link
Contributor

/nephe-test-e2e-agentless

@Anandkumar26 Anandkumar26 force-pushed the account-config branch 3 times, most recently from 6b75a57 to b018cf1 Compare August 8, 2023 14:32
The cloud account config contains the service configs and cloud
api clients. Whenever cloud account config update fails, reset the
cloud account config state to invalid.

Any consumer of cloud account config which make the cloud API calls
should check the cloud account config state and proceed only if
its in a valid state.

Signed-off-by: Anand Kumar <[email protected]>
Consider a scenario where the following sequence of events are received.
An ANP with Ipaddress block and ATGroup is realized -> Secret delete ->
Sync with cloud finds diff and updates member -> AT Modify -> AT Modofy
-> NP delete -> ProcessTrackers -> AT delete.

This results in a scenario where a tracker has one appliedToSgs, there are
no nps in npIndexers, prevAppliedToSgs is nil, computing NP status will
result in an empty appliedToToNpMap and there by an error.

Fix is to add an additional check to compute Npstatus from
appliedToToNpMap and appliedToSgs.

Signed-off-by: Anand Kumar <[email protected]>
@reachjainrahul
Copy link
Contributor

/nephe-test-e2e-agentless

@reachjainrahul
Copy link
Contributor

/nephe-test-e2e-azure

2 similar comments
@reachjainrahul
Copy link
Contributor

/nephe-test-e2e-azure

@reachjainrahul
Copy link
Contributor

/nephe-test-e2e-azure

Copy link
Contributor

@reachjainrahul reachjainrahul left a comment

Choose a reason for hiding this comment

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

/LGTM

@reachjainrahul
Copy link
Contributor

/nephe-test-e2e-azure

@reachjainrahul reachjainrahul merged commit 4cae82e into main Aug 14, 2023
12 checks passed
@reachjainrahul reachjainrahul deleted the account-config branch August 14, 2023 01:43
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