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 wait for K8s resources #82

Merged
merged 17 commits into from
Oct 7, 2020

Conversation

schegi
Copy link
Contributor

@schegi schegi commented Aug 28, 2020

  • Add some convenient methods for actively waiting for K8s resources cluster and nodepool the reach a specified state.
  • Types K8S resource state constants

Bases on: #80

@schegi schegi changed the title Add wait for nodepool Add wait for K8s resources Aug 28, 2020
Copy link
Contributor

@avorima avorima left a comment

Choose a reason for hiding this comment

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

Needs rebase after #80 is merged

go.mod Outdated
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 // indirect
k8s.io/apimachinery v0.18.6
Copy link
Contributor

Choose a reason for hiding this comment

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

No, please don't import apimachinery here. I see you using one function from there, just re-implement it or write an equivalent function.

k8s.go Outdated
"k8s.io/apimachinery/pkg/util/wait"
)

type KubernetesClusterNodePoolState string
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this type

k8s.go Outdated
K8sStateTerminated KubernetesClusterNodePoolState = "TERMINATED"
)

type KubernetesNodeState string
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this type

k8s.go Outdated
func (c *Client) WaitForKubernetesNodePoolState(
clusterID, nodePoolID string,
state KubernetesClusterNodePoolState,
timeout, interval time.Duration) (err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The error returned by this function should include which state was polled

k8s.go Outdated
clusterID, nodePoolID string,
state KubernetesClusterNodePoolState,
timeout, interval time.Duration) (err error) {
return wait.PollImmediate(interval, timeout, func() (done bool, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

These named return values also are not needed

k8s.go Outdated
if err != nil {
return false, err
}
return np != nil && np.Metadata != nil && np.Metadata.State == string(state), err
Copy link
Contributor

Choose a reason for hiding this comment

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

Return nil instead of err here

k8s.go Outdated
}

func (c *Client) WaitForKubernetesClusterState(
clusterID string, state KubernetesClusterNodePoolState, timeout, interval time.Duration) (err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same remarks as above

Copy link
Contributor

@avorima avorima left a comment

Choose a reason for hiding this comment

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

Please run go mod tidy and goimports.

k8s.go Outdated
})
return np != nil && np.Metadata != nil && np.Metadata.State == state, err
}); err != nil {
return fmt.Errorf("error waiting for nodepool state %s [%w]", state, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use : %w for wraps

Copy link
Contributor

@avorima avorima left a comment

Choose a reason for hiding this comment

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

LGTM

@pennycoders pennycoders merged commit 30255d9 into ionos-enterprise:master Oct 7, 2020
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.

3 participants