Skip to content
This repository has been archived by the owner on Oct 30, 2024. It is now read-only.

Add networkpool support on AWS Clusters #577

Merged
merged 4 commits into from
Oct 2, 2020
Merged

Conversation

paurosello
Copy link
Contributor

Checklist

  • Consider SIG UX feedback.
  • Update changelog in CHANGELOG.md.
  • If adding a new type, ensure that it is added to the CRD unit tests.

@paurosello paurosello requested a review from a team October 1, 2020 15:01
@xh3b4sd
Copy link
Contributor

xh3b4sd commented Oct 1, 2020

Some thoughts.

  • Are there some tests we can extend or add?
  • Should we have additional validation?
  • How is the optionality of network pools still ensured?

@paurosello
Copy link
Contributor Author

I created some tests, thanks for reminding 👍

I am checking that setting an empty value for the NetworkPool produces same output as not even specifying the attribute in order to test optionality.

The second test I added is to check that the CR is correctly generated when specifying a NetworkPool

@@ -144,3 +154,86 @@ func newAWSClusterExampleCR() *AWSCluster {

return cr
}

func newAWSClusterEmptyNetworkPoolCR() *AWSCluster {
awscluster := newAWSClusterExampleCR()
Copy link
Contributor

Choose a reason for hiding this comment

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

Just FMI why do we have newAWSClusterExampleCR and NewAWSClusterCR? 🙈

Copy link
Contributor

Choose a reason for hiding this comment

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

I see in

// NewAWSClusterCR returns an AWSCluster Custom Resource.
func NewAWSClusterCR() *AWSCluster {
return &AWSCluster{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
annotation.Docs: awsClusterDocumentationLink,
},
},
TypeMeta: NewAWSClusterTypeMeta(),
}
}
that the spec is empty when using NewAWSClusterCR. /shrug

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, not sure about why do we have that empty CR function

@paurosello paurosello merged commit 99fa9ca into master Oct 2, 2020
@paurosello paurosello deleted the networkpools_awscluster branch October 2, 2020 08:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants