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.

Sign up for GitHub

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

Jump to bottom

Errors while generating sagemaker-controller for ACK runtime v0.16.4 #1161

Closed
ack-bot opened this issue Feb 2, 2022 · 3 comments
Closed

Errors while generating sagemaker-controller for ACK runtime v0.16.4 #1161

ack-bot opened this issue Feb 2, 2022 · 3 comments
Labels
ack-bot-autogen

Comments

@ack-bot
Copy link
Collaborator

ack-bot commented Feb 2, 2022

Errors while generating sagemaker-controller for ACK runtime v0.16.4

stdout for make build-controller:

building ack-generate ... ok.
==== building sagemaker-controller ====
Copying common custom resource definitions into sagemaker
Building Kubernetes API objects for sagemaker
Generating deepcopy code for sagemaker

stderr for make build-controller:

github.com/aws-controllers-k8s/sagemaker-controller/apis/v1alpha1:-: invalid pointer element type: invalid type
Error: not all generators ran successfully
run `controller-gen object:headerFile=/home/prow/go/src/github.com/aws-controllers-k8s/code-generator/scripts/../templates/boilerplate.txt paths=./... -w` to see all available markers, or `controller-gen object:headerFile=/home/prow/go/src/github.com/aws-controllers-k8s/code-generator/scripts/../templates/boilerplate.txt paths=./... -h` for usage
make: *** [Makefile:34: build-controller] Error 1

Steps for closing this issue:

  1. Update go.mod file to update aws-controllers-k8s/runtime to v0.16.4
  2. Successfully generate the service controller locally using latest release of
    aws-controllers-k8s/code-generator
  3. Run go mod tidy for sagemaker-controller
  4. Run make test for sagemaker-controller
  5. Run make kind-test from aws-controllers-k8s/test-infra repository for
    sagemaker-controller
  6. When kind test is successful, Create new pull request for sagemaker-controller
  7. Mention this issue number in the pull request
  8. When the above PR is merged, close this issue

For more guidance, create an issue
or reach out on Slack.
@vijtrip2
Copy link
Contributor

vijtrip2 commented Feb 2, 2022

This failed because of a new empty struct "RSessionAppSettings" in API Model. After ignoring that struct code-generation works fine for sagemaker controller with v0.16.4

I will keep this issue on hold and first update sagemaker controller from v0.15.2 to v0.16.3, then work on this issue

@vijtrip2 vijtrip2 mentioned this issue Feb 3, 2022
Merged
ack-bot pushed a commit to aws-controllers-k8s/runtime that referenced this issue Feb 7, 2022
@vijtrip2
Use original status when making api-server Status.Update call in ad…
65b9179 
…option reconciler (#71)

Issue #, if available: aws-controllers-k8s/community#1161

Description of changes:
* The Status update from AdoptionReconciler was not useful because the Create call before Status.Update was resetting status of the CustomResource.
* Earlier this similar kind of problem was also present in `reconciler.go` and it was fixed by keeping an original copy of Status before making Create/Update calls
* I used the same solution in AdoptionReconciler.

----------

* Discovered this issue while debugging SageMaker ModelPackage adoption test. ModelPackage uses ARN as identifier which needs to be set inside `Status.ACKResourceMetadata.ARN` field of CustomResource.
* Since the Identifier was never getting correctly set, the `ReadOne` call inside `reconciler.go` was failing while reconciling the CustomResource.

--------------

* Validated by running SageMaker end-to-end tests successfully.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
@vijtrip2 vijtrip2 mentioned this issue Feb 8, 2022
Merged
ack-bot pushed a commit to aws-controllers-k8s/code-generator that referenced this issue Feb 8, 2022
@vijtrip2
Update ACK runtime from v0.16.4 => v0.16.5 (#286)
edec6da 
Issue #, if available: aws-controllers-k8s/community#1161

Description of changes:

* ACK runtime from v0.16.4 to v0.16.5
* Release notes: https://github.com/aws-controllers-k8s/runtime/releases/tag/v0.16.5 

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

/close

duplicate of #1167

@ack-bot
Copy link
Collaborator Author

ack-bot commented Feb 14, 2022

@vijtrip2: Closing this issue.

In response to this:

/close

duplicate of #1167

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ack-bot ack-bot closed this as completed Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ack-bot-autogen
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@vijtrip2 @ack-bot