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

REST API for VPC #80

Merged
merged 6 commits into from
Jan 12, 2023
Merged

REST API for VPC #80

merged 6 commits into from
Jan 12, 2023

Conversation

bangqipropel
Copy link
Contributor

  • Add API for VPC struct
  • Expose VPC struct in apiserver
  • Add unit test for the changes introduced

@bangqipropel bangqipropel changed the base branch from main to vpc_poller December 14, 2022 07:48
@bangqipropel
Copy link
Contributor Author

more unit tests can be added tomorrow, and for now, we just compare this branch to vpc_poller now

@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Merging #80 (0c6c673) into main (34fe66e) will increase coverage by 0.26%.
The diff coverage is 74.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #80      +/-   ##
==========================================
+ Coverage   55.70%   55.97%   +0.26%     
==========================================
  Files          48       49       +1     
  Lines        7691     7804     +113     
==========================================
+ Hits         4284     4368      +84     
- Misses       2977     2997      +20     
- Partials      430      439       +9     
Flag Coverage Δ
unit-tests 55.97% <74.33%> (+0.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/apiserver/registry/inventory/rest.go 74.33% <74.33%> (ø)

pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/vpc_test.go Outdated Show resolved Hide resolved
func (r *REST) Get(ctx context.Context, name string, _ *metav1.GetOptions) (runtime.Object, error) {
ns, ok := request.NamespaceFrom(ctx)
if !ok || len(ns) == 0 {
return nil, errors.NewBadRequest("Namespace cannot be empty.")
Copy link
Contributor

@Anandkumar26 Anandkumar26 Dec 16, 2022

Choose a reason for hiding this comment

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

can the namespace ever be empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think when it has some error when requesting from the server, it an be this

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: namespace

return nil, errors.NewBadRequest("Namespace cannot be empty.")
}

objs, found := r.vpcIndexer.ByIndex(cloud.VpcIndexerByName, name)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
objs, found := r.vpcIndexer.ByIndex(cloud.VpcIndexerByName, name)
objs, err := r.vpcIndexer.ByIndex(cloud.VpcIndexerByName, name)

pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/rest.go Outdated Show resolved Hide resolved
pkg/apiserver/registry/vpc/vpc_test.go Outdated Show resolved Hide resolved
archanapholla and others added 4 commits January 11, 2023 15:04
- Perform cloud inventory poll for vpcs on CPA add, vm instanes are skipped
  as configured filters are nil.
- Move account poller(fetches cloud data from internal snapshot) to CPA.
- Account poller is applicable for both vpc and vm instance purpose.
- Maintain vpc list globally using cache indexers.
- In CES, use existing account poller(created during CPA add).
- On CES delete, reset vmSelector filters and skip inventory poll for vms,
  do not distub inventory poll for vpcs.
- On CPA delete, stop cloud inventory poll and remove account poller.
- Add unit test for the changes introduced.
- Add documentation for vpc poller.

Signed-off-by: Archana Holla <[email protected]>
- Fix for integration test issues
- Improve logging.
- User lower case for some fields in VPC object(follow VM CRD format).

Signed-off-by: Archana Holla <[email protected]>
- Also remove unwanted files.

Signed-off-by: Anand Kumar <[email protected]>
Signed-off-by: Bangqi Zhu <[email protected]>
@reachjainrahul reachjainrahul changed the base branch from vpc_poller to main January 12, 2023 13:21
Resolving conflicts
 Conflicts:
	apis/runtime/v1alpha1/vpc_types.go
	pkg/controllers/cloud/account_poller.go
	pkg/controllers/cloud/cloudprovideraccount_controller.go
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-aws

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 reachjainrahul merged commit 0e33985 into main Jan 12, 2023
@reachjainrahul reachjainrahul deleted the vpc_REST branch January 13, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants