-
Notifications
You must be signed in to change notification settings - Fork 71
Allow to create multiple service networks #113
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
controller in loops
| glog.V(6).Infof("updateGatewayStatus: updating last transition time \n") | ||
| gw.Status.Conditions[0].LastTransitionTime = metav1.NewTime(time.Now()) | ||
| if gw.Status.Conditions[0].LastTransitionTime == eventhandlers.ZeroTransitionTime { | ||
| gw.Status.Conditions[0].LastTransitionTime = metav1.NewTime(time.Now()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic seems wrong. Don't you want to change this every time the condition transitions? Right now, it only checks when it goes from unset -> set. Is it ever possible for this condition to be false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we reset it whenever there is a change and need controller to reconcile it
| case vpclattice.ServiceNetworkVpcAssociationStatusDeleteInProgress: | ||
| return latticemodel.ServiceNetworkStatus{ServiceNetworkARN: "", ServiceNetworkID: ""}, errors.New(LATTICE_RETRY) | ||
| } else { | ||
| if isServiceNetworkAssociatedWithVPC == true { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get the idea, that if service_network.Spec.AssociateToVPC=false, and ServiceNetwork is currently AssociatedWithVPC, we want to disassociate vpcAndServiceNetwork. I feel it is a bit wired that this is doing delete work while in create section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, the associationToVPC or disassociatefromVPC is kind of like an attribute of a service-network/gateway here. The K8S intend is to create gateway==servicenetwork, with attribute whether to associate/dis-associate to VPC
Issue #73 , if available:
Description of changes:
Here the description on this enhancement
Here are all the updated unit tests and manual end-to-end test(s) we have done
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.