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

Move VM CRD to VM Runtime Object #167

Merged
merged 8 commits into from
Mar 29, 2023
Merged

Move VM CRD to VM Runtime Object #167

merged 8 commits into from
Mar 29, 2023

Conversation

reachjainrahul
Copy link
Contributor

Depcreate VM CRD and move it to internal VM Runtime Object.

serviceConfigs := accCfg.GetServiceConfigs()
for _, serviceConfig := range serviceConfigs {
if serviceConfig.getType() == CloudServiceTypeCompute {
resourceCRDs := serviceConfig.getResourceCRDs(namespace, accCfg.GetNamespacedName().String())
computeCRDs = append(computeCRDs, resourceCRDs.virtualMachines...)
for _, vm := range resourceCRDs.virtualMachines {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can create map inside getResourceCRDs itself.. Line 169 can be avoided.. @archanapholla can you make the change

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes done.

@Anandkumar26 Anandkumar26 force-pushed the vmcrd branch 3 times, most recently from df93005 to 1570022 Compare March 21, 2023 16:14
namespace, _ := request.NamespaceFrom(ctx)
if namespace == "" {
objs = r.cloudInventory.GetAllVms()
} else if accountName != "" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with new change, it should be cpa-name + cpa-namespace.. both label should be specified..

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

r.pendingSyncCount--
if r.pendingSyncCount == 0 {
GetControllerSyncStatusInstance().SetControllerSyncStatus(ControllerTypeVM)
// processEvent waits for any VM events and feeds the event to the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lets put 80 max character line for any comments

Copy link
Contributor

Choose a reason for hiding this comment

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

Will make this as a standard going forward.

@@ -184,8 +184,11 @@ var _ = Describe(fmt.Sprintf("%s,%s: ExternalEntity", focusAws, focusAzure), fun
}
table.DescribeTable("Test ExternalEntity Life cycle",
func(kind string, hasNic, hasPort bool) {
// TODO: VM CRD is no longer present
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shenmo3 can you update the test similar to stale member test. Upon Ces and check if VM/EE is removed

@Anandkumar26 Anandkumar26 force-pushed the vmcrd branch 4 times, most recently from bf330f8 to 9131411 Compare March 29, 2023 14:05
reachjainrahul and others added 8 commits March 29, 2023 07:05
Depcreate VM CRD and move it to internal VM Runtime Object.

Signed-off-by: Rahul Jain <[email protected]>
- Also sort imports
- Update logging for the controller_sync_status.go
- Fix lint errors

Signed-off-by: Anand Kumar <[email protected]>
- Also remove unwanted labels from VM object.
- Add and update existing VMIndexers to use labels
and new fields added in the vm status object.
- Also update vpc label to use account namespaced name instead
of using only the account name.

Signed-off-by: Anand Kumar <[email protected]>
- Elimimate CloudServiceResourceCRDs while fetching VirtualMachine
objects from snapshot.
- Add counters for add/update/delete in vm inventory.
- Renamed functions/variables using the word vm CRD.
- Eliminate member by member comparison for comparing vm objects from inventory
with new objects created using cloud vm instances.

Signed-off-by: Archana Holla <[email protected]>
vpc labels:
cpa.name
cpa.namespace
vpc.name

vm labels:
cpa.name
cpa.namespace
region

Also update rest implmentation to list objects based on label
selector for vm and vpc.

- Also remove vpc name from vm status field.
- rename some vm status feilds
- Also update vm inventory to use namespaced account name indexer

Signed-off-by: Anand Kumar <[email protected]>
- Remove virtual machine crd form helm charts
- Remove virtual machine crd from config

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

/nephe-test-e2e-kind

Copy link
Contributor Author

@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

Copy link
Contributor

@Anandkumar26 Anandkumar26 left a comment

Choose a reason for hiding this comment

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

/LGTM

@Anandkumar26
Copy link
Contributor

/nephe-test-e2e-aks, /nephe-test-e2e-eks

@reachjainrahul
Copy link
Contributor Author

/nephe-test-e2e-azure

@reachjainrahul reachjainrahul merged commit 9959e39 into main Mar 29, 2023
@reachjainrahul reachjainrahul deleted the vmcrd branch June 2, 2023 15:56
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.

4 participants