Skip to content

Conversation

@knottnt
Copy link
Contributor

@knottnt knottnt commented Oct 10, 2025

Issue #, if available: 2581

Description of changes:

  • Generate CRD and controller for Bedrock Inference Profile
  • Add hooks for setting ARN to InferenceProfileIdentifier in ReadOne and Delete operations
  • Add custom update for applying tag changes.
  • Add e2e test to verify CRUD operations

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

Add .vscode
- Ignore all resources except for InferenceProfile
- Add hooks for setting InferenceProfileIdentifier with ARN
- Set non-tag fields as immutable
- Add custom update method to update tags if a delta is detected
- Add hooks for read and updating tags
- Update generator.yaml to add InferenceProfileID from GetInferenceProfile operation
@ack-prow ack-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 10, 2025
@ack-prow ack-prow bot requested review from a-hilaly and rushmash91 October 10, 2025 17:43
@ack-prow ack-prow bot added the approved label Oct 10, 2025
@knottnt
Copy link
Contributor Author

knottnt commented Oct 13, 2025

/test all

@knottnt
Copy link
Contributor Author

knottnt commented Oct 13, 2025

/retest

@knottnt knottnt marked this pull request as ready for review October 13, 2025 16:49
@ack-prow ack-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 13, 2025
@ack-prow ack-prow bot requested a review from michaelhtm October 13, 2025 16:49
@ack-prow
Copy link

ack-prow bot commented Oct 13, 2025

@knottnt: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
bedrock-verify-attribution 0cab921 link false /test bedrock-verify-attribution

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

Thank you @knottnt ! approved with one nit


// custom_update handles updates for inference profiles. Since inference profiles
// are immutable resources, only tag updates are supported.
func (rm *resourceManager) custom_update(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func (rm *resourceManager) custom_update(
func (rm *resourceManager) customUpdate(

return nil, err
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Can we also support the other update paths or is this an immutable resource?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, inference profile doesn't currently have an update operation.

Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Oct 15, 2025
@ack-prow
Copy link

ack-prow bot commented Oct 15, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-hilaly, knottnt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-prow ack-prow bot merged commit 54e6965 into aws-controllers-k8s:main Oct 15, 2025
6 of 7 checks passed
if err != nil {
return nil, err
}
return &resource{ko}, nil
Copy link
Member

Choose a reason for hiding this comment

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

@knottnt looks like we need a requeue when status is CREATING?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh will add a synced when config to generator.yaml

Copy link
Member

Choose a reason for hiding this comment

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

please move to the outer folder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By outer folder do you mean /pkg/tags/sync.go?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants