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

Feature - Add/Modify/Delete tags for model resource #261

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Feature - Add/Modify/Delete tags for model resource #261

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f5b80a3
regenerated code again
rkurduka Mar 1, 2024
d5ca8ae
added a hook to handle modeldatasource updates
rkurduka Mar 1, 2024
63768ef
fixed update modeldatasource issue in sdkfind
rkurduka Mar 1, 2024
0e99f49
fixed versioning for code-generator and on helm values
rkurduka Mar 5, 2024
9eca23c
added check to validate update field
rkurduka Mar 6, 2024
99037c2
added check to validate field for update
rkurduka Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions apis/v1alpha1/ack-generate-metadata.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
ack_generate_info:
build_date: "2024-02-24T00:04:37Z"
build_hash: f429bd95efc1c7286e7cc973dc174a30490a5521
go_version: go1.22.0
version: v0.30.0-3-gf429bd9
build_date: "2024-03-01T09:24:39Z"
build_hash: c2165b65565ab3a094cfb474c4396f4d14c7ef1e
go_version: go1.21.6
version: v0.27.1-55-gc2165b6
api_directory_checksum: 731faf4c5d6d6f5140b4e0786127df447f773217
api_version: v1alpha1
aws_sdk_go_version: v1.50.15
generator_config_info:
file_checksum: 0d728ab3662c7e538aff6727f087b54c5969fdcf
file_checksum: 43c8931df528f38bf2c179722e7878f4615d5584
original_file_name: generator.yaml
last_modification:
reason: API generation
17 changes: 14 additions & 3 deletions apis/v1alpha1/generator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,22 @@ operations:
StopPipelineExecution:
operation_type: Delete
resource_name: PipelineExecution
AddTags:
operation_type: Update
resource_name: Model
Comment on lines +25 to +27
Copy link
Member

Choose a reason for hiding this comment

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

This generates code that treats AddTags as the Update API Call to update the model.

Copy link
Author

Choose a reason for hiding this comment

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

@a-hilaly - Thanks for review.
Sagemaker model API do not have any modify /update method , as fields are immutable except tags . Also Sagemaker API resource only have "ADDTAG" and "DELETE TAG" . So to handle update on this resource will have to use one of these two. So chose "ADD TAGS" and handling "DELETE tags" using template hooks.

Let me know if you think any other approach we can consider

Copy link
Member

Choose a reason for hiding this comment

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

Agree with Amine, need to discuss this further as we need to throw appropriate error if any other fields except Tags are modified as Model does not have udpate API, maybe implement it completely via hooks

Copy link
Author

@rkurduka rkurduka Mar 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@surajkota - Added check in update_pre hooks to validate field to update, if it is other than tag , reconciler will update "message" status. Please review

Please let me know if you feel anything else is require

resources:
Model:
hooks:
delta_pre_compare:
code: customSetDefaults(a, b)
sdk_read_one_pre_set_output:
template_path: model/sdk_read_one_pre_set_output.go.tpl
sdk_read_one_post_set_output:
template_path: model/sdk_read_one_post_set_output.go.tpl
sdk_update_pre_build_request:
template_path: model/sdk_update_pre_build_request.go.tpl
sdk_update_post_build_request:
template_path: model/sdk_update_post_build_request.go.tpl
exceptions:
errors:
404:
Expand All @@ -37,9 +48,9 @@ resources:
- InvalidParameterValue
- MissingParameter
fields:
Tags:
compare:
is_ignored: true
# Tags:
# compare:
# is_ignored: true
EnableNetworkIsolation:
late_initialize:
min_backoff_seconds: 5
Expand Down
16 changes: 16 additions & 0 deletions cmd/controller/main.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions config/controller/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,18 @@ spec:
capabilities:
drop:
- ALL
livenessProbe:
httpGet:
path: /healthz
port: 8081
initialDelaySeconds: 15
periodSeconds: 20
readinessProbe:
httpGet:
path: /readyz
port: 8081
initialDelaySeconds: 5
periodSeconds: 10
securityContext:
seccompProfile:
type: RuntimeDefault
Expand Down
2 changes: 1 addition & 1 deletion config/controller/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ kind: Kustomization
images:
- name: controller
newName: public.ecr.aws/aws-controllers-k8s/sagemaker-controller
newTag: 1.2.7
newTag: 0.0.0-non-release-version
206 changes: 89 additions & 117 deletions config/crd/common/bases/services.k8s.aws_adoptedresources.yaml
View file
Open in desktop

Large diffs are not rendered by default.

54 changes: 22 additions & 32 deletions config/crd/common/bases/services.k8s.aws_fieldexports.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.14.0
controller-gen.kubebuilder.io/version: v0.9.2
Copy link
Member

Choose a reason for hiding this comment

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

You need to install controller-gen v0.14.0

Copy link
Author

Choose a reason for hiding this comment

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

Done

creationTimestamp: null
name: fieldexports.services.k8s.aws
spec:
group: services.k8s.aws
Expand All @@ -20,37 +21,30 @@ spec:
description: FieldExport is the schema for the FieldExport API.
properties:
apiVersion:
description: |-
APIVersion defines the versioned schema of this representation of an object.
Servers should convert recognized schemas to the latest internal value, and
may reject unrecognized values.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
description: 'APIVersion defines the versioned schema of this representation
of an object. Servers should convert recognized schemas to the latest
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
kind:
description: |-
Kind is a string value representing the REST resource this object represents.
Servers may infer this from the endpoint the client submits requests to.
Cannot be updated.
In CamelCase.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
description: 'Kind is a string value representing the REST resource this
object represents. Servers may infer this from the endpoint the client
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
type: string
metadata:
type: object
spec:
description: FieldExportSpec defines the desired state of the FieldExport.
properties:
from:
description: |-
ResourceFieldSelector provides the values necessary to identify an individual
field on an individual K8s resource.
description: ResourceFieldSelector provides the values necessary to
identify an individual field on an individual K8s resource.
properties:
path:
type: string
resource:
description: |-
NamespacedResource provides all the values necessary to identify an ACK
resource of a given type (within the same namespace as the custom resource
containing this type).
description: NamespacedResource provides all the values necessary
to identify an ACK resource of a given type (within the same
namespace as the custom resource containing this type).
properties:
group:
type: string
Expand All @@ -68,18 +62,16 @@ spec:
- resource
type: object
to:
description: |-
FieldExportTarget provides the values necessary to identify the
output path for a field export.
description: FieldExportTarget provides the values necessary to identify
the output path for a field export.
properties:
key:
description: Key overrides the default value (`<namespace>.<FieldExport-resource-name>`)
for the FieldExport target
type: string
kind:
description: |-
FieldExportOutputType represents all types that can be produced by a field
export operation
description: FieldExportOutputType represents all types that can
be produced by a field export operation
enum:
- configmap
- secret
Expand All @@ -102,14 +94,12 @@ spec:
description: FieldExportStatus defines the observed status of the FieldExport.
properties:
conditions:
description: |-
A collection of `ackv1alpha1.Condition` objects that describe the various
recoverable states of the field CR
description: A collection of `ackv1alpha1.Condition` objects that
describe the various recoverable states of the field CR
items:
description: |-
Condition is the common struct used by all CRDs managed by ACK service
controllers to indicate terminal states of the CR and its backend AWS
service API resource
description: Condition is the common struct used by all CRDs managed
by ACK service controllers to indicate terminal states of the
CR and its backend AWS service API resource
properties:
lastTransitionTime:
description: Last time the condition transitioned from one status
Expand Down
17 changes: 14 additions & 3 deletions generator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,22 @@ operations:
StopPipelineExecution:
operation_type: Delete
resource_name: PipelineExecution
AddTags:
operation_type: Update
resource_name: Model
resources:
Model:
hooks:
delta_pre_compare:
code: customSetDefaults(a, b)
sdk_read_one_pre_set_output:
template_path: model/sdk_read_one_pre_set_output.go.tpl
sdk_read_one_post_set_output:
template_path: model/sdk_read_one_post_set_output.go.tpl
sdk_update_pre_build_request:
template_path: model/sdk_update_pre_build_request.go.tpl
sdk_update_post_build_request:
template_path: model/sdk_update_post_build_request.go.tpl
exceptions:
errors:
404:
Expand All @@ -37,9 +48,9 @@ resources:
- InvalidParameterValue
- MissingParameter
fields:
Tags:
compare:
is_ignored: true
# Tags:
# compare:
# is_ignored: true
EnableNetworkIsolation:
late_initialize:
min_backoff_seconds: 5
Expand Down
4 changes: 2 additions & 2 deletions helm/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
apiVersion: v1
name: sagemaker-chart
description: A Helm chart for the ACK service controller for Amazon SageMaker (SageMaker)
version: 1.2.7
appVersion: 1.2.7
version: 0.0.0-non-release-version
Copy link
Member

Choose a reason for hiding this comment

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

Please pull the controller tags

Copy link
Author

Choose a reason for hiding this comment

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

Done

appVersion: 0.0.0-non-release-version
home: https://github.com/aws-controllers-k8s/sagemaker-controller
icon: https://raw.githubusercontent.com/aws/eks-charts/master/docs/logo/aws.png
sources:
Expand Down
4 changes: 2 additions & 2 deletions helm/crds/services.k8s.aws_adoptedresources.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,12 @@ spec:
name:
description: |-
Name of the referent.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names#names
More info: http://kubernetes.io/docs/user-guide/identifiers#names
type: string
uid:
description: |-
UID of the referent.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names#uids
More info: http://kubernetes.io/docs/user-guide/identifiers#uids
type: string
required:
- apiVersion
Expand Down
2 changes: 1 addition & 1 deletion helm/templates/NOTES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{{ .Chart.Name }} has been installed.
This chart deploys "public.ecr.aws/aws-controllers-k8s/sagemaker-controller:1.2.7".
This chart deploys "public.ecr.aws/aws-controllers-k8s/sagemaker-controller:0.0.0-non-release-version".

Check its status by running:
kubectl --namespace {{ .Release.Namespace }} get pods -l "app.kubernetes.io/instance={{ .Release.Name }}"
Expand Down
12 changes: 12 additions & 0 deletions helm/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,18 @@ spec:
capabilities:
drop:
- ALL
livenessProbe:
httpGet:
path: /healthz
port: 8081
initialDelaySeconds: 15
periodSeconds: 20
readinessProbe:
httpGet:
path: /readyz
port: 8081
initialDelaySeconds: 5
periodSeconds: 10
securityContext:
seccompProfile:
type: RuntimeDefault
Expand Down
2 changes: 1 addition & 1 deletion helm/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

image:
repository: public.ecr.aws/aws-controllers-k8s/sagemaker-controller
tag: 1.2.7
tag: 0.0.0-non-release-version
pullPolicy: IfNotPresent
pullSecrets: []

Expand Down
3 changes: 3 additions & 0 deletions pkg/resource/model/delta.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading