Skip to content

Remove subnet UserIDGroupPairs.GroupID references #192

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.

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

Conversation

a-hilaly
Copy link
Member

@a-hilaly a-hilaly commented May 31, 2024

Partial revert of cd75ad9

Initially our intention was to generate code to reference for both GroupName
and GroupID fields in the UserIDGroupPairs. However what we (I) missed was
that the code-generator processes the references fields, omits "ID" and "Name"
name fields, and infers names for the referenced fields (e.g GroupName ->
GroupRef and GroupID -> GroupRef. This confused the ACK code
generator since both GroupName and GroupID needed a reference field called
GroupRef. This caused some confusion at the code generation level, triggering some
validation issues and introduced a regression in our references code.

In a attempt to address the regression, we will first omit one of the references
(GroupID), regenerate the code and address this in the code-generator later on.

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

@ack-prow ack-prow bot requested review from jljaco and vijtrip2 May 31, 2024 06:32
@ack-prow ack-prow bot added the approved label May 31, 2024
@a-hilaly a-hilaly force-pushed the partial-revert/cd75ad971e604ba0e1f05a4e2f7838cd20baea79 branch 2 times, most recently from 0d673ea to 6fb4468 Compare May 31, 2024 06:35
…ollers-k8s#191)

Issue : aws-controllers-k8s/community#2075

Description of changes:
The issue is that vpce CR status is not updated. This happens because when vpce is created, it takes around a minute for it to be created in aws. Due to this, it's status is kept `pending`. But there is no sync enforced on the reconciler. Due to this, reconciliation happens after 10h default time. At this time, status would be set correctly.

This fix enforces reconciliation to make sure CR is updated correctly when vpce creation in aws is successful.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
@a-hilaly a-hilaly force-pushed the partial-revert/cd75ad971e604ba0e1f05a4e2f7838cd20baea79 branch from 6fb4468 to e883ea9 Compare May 31, 2024 06:37
@ack-bot
Copy link
Collaborator

ack-bot commented May 31, 2024

/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label May 31, 2024
Copy link

ack-prow bot commented May 31, 2024

[APPROVALNOTIFIER] This PR is APPROVED

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

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 63881fe into aws-controllers-k8s:main May 31, 2024
6 checks passed
nnbu pushed a commit to nnbu/ack-ec2-controller that referenced this pull request Sep 18, 2024
…k8s#192)

Partial revert of aws-controllers-k8s@cd75ad9

Initially our intention was to generate code to reference for both `GroupName`
and `GroupID` fields in the `UserIDGroupPairs`. However what we (I) missed was
that the code-generator processes the references fields, omits "ID" and "Name"
name fields, and infers names for the referenced fields (e.g `GroupName` ->
`GroupRef` and `GroupID` -> `GroupRef`. This confused the ACK code
generator since both `GroupName` and `GroupID` needed a reference field called
`GroupRef`. This caused some confusion at the code generation level, triggering some
validation issues and introduced a regression in our references code.

In a attempt to address the regression, we will first omit one of the references
(GroupID), regenerate the code and address this in the code-generator later on.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
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