Refactor pkg/cloud/metadata.go into pkg/cloud/metadata_*.go files #1074
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wongma7 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
wongma7
force-pushed
the
metadatarefactorround2
branch
from
78721e3 to
1d5b5f1
Compare
| "k8s.io/client-go/rest" | ||
| ) | ||
|
|
||
| type KubernetesAPIClient func() (kubernetes.Interface, error) |
There was a problem hiding this comment.
nit: What's the reason for the metadata_ prefix for all files? This seems to just be a Kubernetes client wrapper, maybe client.go or kubernetes.go makes more sense.
There was a problem hiding this comment.
Otherwise lgtm, will merge if you prefer that name.
There was a problem hiding this comment.
the main purpose is for KubernetesAPIInstanceInfo to return a Metadata, i.e. how to retrieve Metadata/InstanceInfo from k8s versus from ec2. (and later, one to retieve from ECS for fargate environment). the function you are looking at is just wrapper stuff for testing, KubernetesAPIInstanceInfo is the important one.
There was a problem hiding this comment.
Yeah that makes sense and I see the reasoning for the similar file names, especially given they all are different mechanisms for finding the same information.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
Is this a bug fix or adding new feature? refactor
What is this PR about? / Why do we need it? I am planning to consume or copy pkg/cloud/metadata_* code from here to the EFS repo so I wanted to split up metadata.go and cloud_interface.go to make it easier. this is justa refactor for readability and easier consumption it should have no consequences on functionality
What testing is done? make mockgen succeeded, will let CI test the rest