Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

Update dependency versions #129

Merged
merged 6 commits into from
Aug 7, 2020

Conversation

RedbackThomson
Copy link
Contributor

@RedbackThomson RedbackThomson commented Aug 3, 2020

What does this PR do / how does this improve the operators?

Updates the following Go dependencies:

  • controller-gen: v0.2.0-beta.2 -> v0.3.0
  • aws-sdk-go-v2: v0.20.0 -> v0.24.0
  • controller-runtime: v0.2.0 -> v0.6.1
  • client-go: v11.0.1-# -> v0.18.6

Which issue(s) does this PR fix?

Fixes #125

Special notes for the reviewer:

Had to make a small change to each of the suite_test.go files (in /api/* and /controllers/*) because the NewLineReporter moved to the envtest/printer directory. Also had to create a new unit test helper method CreateMockNamespace which creates a namespace within the mock API server. It seems that the newer version of the controller-runtime require the namespace be created before attempting to use it. All make targets work properly.

Does this PR require changes to documentation?

No

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you written or refactored unit tests to cover the change?
  • Have you ran all unit tests and ensured they are passing?
  • Have you manually tested each feature that is being added/modified?
  • Have you ensured you have not introduced linting errors?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -56,7 +56,7 @@ type BatchTransformJobSpec struct {
Region *string `json:"region"`

// A custom SageMaker endpoint to use when communicating with SageMaker.
// +kubebuilder:validation:Pattern=^(https|http)://.*$
// +kubebuilder:validation:Pattern="^(https|http)://.*$"
Copy link
Contributor

Choose a reason for hiding this comment

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

Were you able to verify that was actually a version mismatch issue ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. This error was exactly as predicted.

@@ -14,3 +12,5 @@ images:
newTag: v1
resources:
- ../manager
patchesStrategicMerge:
- manager_auth_proxy_patch.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

why the change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This actually comes from @akartsky 's latest changes. I switched the order of his make targets so that it runs the US target last, that way all of our Kustomize templates have the US implementation as the default when checked in.

Copy link
Contributor

Choose a reason for hiding this comment

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

wow, good thinking.

@mbaijal
Copy link
Contributor

mbaijal commented Aug 6, 2020

We should probably call out these versions in the docs or atleast the READMEs too

@RedbackThomson
Copy link
Contributor Author

We should probably call out these versions in the docs or atleast the READMEs too

These versions are specified in the go.mod file? Which versions would you otherwise want to explicitly state?

@mbaijal
Copy link
Contributor

mbaijal commented Aug 6, 2020

We should probably call out these versions in the docs or atleast the READMEs too

These versions are specified in the go.mod file? Which versions would you otherwise want to explicitly state?

I guess that's true. If someone is developing on top of these operators they would be familiar enough with golang to know where to look.

@RedbackThomson RedbackThomson merged commit fcad3ca into aws:master Aug 7, 2020
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.

SageMaker Operator Types fails KubeBuilder Pattern validation check
2 participants