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.

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

init gateway api #328

Merged
merged 6 commits into from
Jan 16, 2024
Merged

init gateway api #328

merged 6 commits into from
Jan 16, 2024

Conversation

Abdiramen
Copy link
Collaborator

What

Describe what the change is solving

How

Describe the solution

Breaking Changes

Are there any breaking changes in this PR?

@Abdiramen
Copy link
Collaborator Author

Abdiramen commented Dec 28, 2023

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@Abdiramen Abdiramen mentioned this pull request Dec 28, 2023
@github-actions github-actions bot added area/controller Issues dealing with the controller area/helm-chart Issues dealing with the helm chart and removed area/helm-chart Issues dealing with the helm chart labels Dec 28, 2023
@Abdiramen Abdiramen force-pushed the oz/gateway-initialize branch 2 times, most recently from a677210 to 93b1cab Compare January 8, 2024 16:27
@github-actions github-actions bot added the area/helm-chart Issues dealing with the helm chart label Jan 8, 2024
@Abdiramen Abdiramen force-pushed the oz/gateway-initialize branch from 93b1cab to e2f76dd Compare January 8, 2024 17:19
@Abdiramen Abdiramen marked this pull request as ready for review January 8, 2024 19:21
@Abdiramen Abdiramen requested a review from a team as a code owner January 8, 2024 19:21
@Abdiramen Abdiramen marked this pull request as draft January 8, 2024 19:22
@Abdiramen Abdiramen marked this pull request as ready for review January 8, 2024 23:21
internal/store/driver.go Outdated Show resolved Hide resolved
log.V(1).Info("reconciliation triggered but gateway does not exist, ingnoring")
return ctrl.Result{Requeue: false}, nil
}
return ctrl.Result{Requeue: true}, nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

if the error != nil, but its not a not found error, requeueing makes sense, but should we log or send an event?


log.V(1).Info("verifying gatewayclass")
gwClass := &gatewayv1.GatewayClass{}
if err := r.Client.Get(ctx, client.ObjectKey{Name: string(gw.Spec.GatewayClassName)}, gwClass); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is a gateway class name required? just based on the ingress class, it was optional and you could register a default with the cluster. The way the ingress controller work FYI is the driver/store gathers all the ingress objects and then the driver calculation functions reference https://github.com/ngrok/kubernetes-ingress-controller/blob/main/internal/store/store.go#L319-L336 to get the filtered lists of ingress object that should be handled based on ingress class filtering. There are also other ways we could have handled this like filtering before inserting into the cache, but just a heads up about the pattern here incase you want to apply it

@Abdiramen Abdiramen force-pushed the oz/kubebuilder-multigroup branch from adb4efd to a26ca96 Compare January 10, 2024 17:11
@Abdiramen Abdiramen force-pushed the oz/gateway-initialize branch from 5bcbad9 to 2aa8aaf Compare January 10, 2024 17:11
@Abdiramen Abdiramen force-pushed the oz/kubebuilder-multigroup branch from a26ca96 to e09cba5 Compare January 10, 2024 19:27
@Abdiramen Abdiramen force-pushed the oz/gateway-initialize branch from de9ea68 to c3f901e Compare January 10, 2024 19:27
Base automatically changed from oz/kubebuilder-multigroup to main January 11, 2024 16:01
@Abdiramen Abdiramen force-pushed the oz/gateway-initialize branch 2 times, most recently from 3d41cdf to 9e26c62 Compare January 12, 2024 18:47
@github-actions github-actions bot added the area/ci Issues/PRs relating to CI label Jan 12, 2024
Abdiramen and others added 2 commits January 12, 2024 12:49
@Abdiramen Abdiramen force-pushed the oz/gateway-initialize branch from 9e26c62 to 8789113 Compare January 12, 2024 18:49
@Abdiramen Abdiramen merged commit 852b642 into main Jan 16, 2024
14 checks passed
@Abdiramen Abdiramen deleted the oz/gateway-initialize branch January 16, 2024 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci Issues/PRs relating to CI area/controller Issues dealing with the controller area/helm-chart Issues dealing with the helm chart
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants