diff --git a/internal/manager/manager.go b/internal/manager/manager.go index a2bfcda03f..520d932c89 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -70,7 +70,6 @@ func Start(cfg config.Config) error { // Note: for any new object type or a change to the existing one, // make sure to also update prepareFirstEventBatchPreparerArgs() - // FIXME(pleshakov): Make the comment above redundant. controllerRegCfgs := []struct { objectType client.Object options []controller.Option diff --git a/internal/status/gateway.go b/internal/status/gateway.go index 349785de01..abb37e3098 100644 --- a/internal/status/gateway.go +++ b/internal/status/gateway.go @@ -10,9 +10,6 @@ import ( ) // prepareGatewayStatus prepares the status for a Gateway resource. -// FIXME(pleshakov): Be compliant with in the Gateway API. -// Currently, we only support simple valid/invalid status per each listener. -// Extend support to cover more cases. func prepareGatewayStatus( gatewayStatus state.GatewayStatus, podIP string, @@ -21,6 +18,7 @@ func prepareGatewayStatus( listenerStatuses := make([]v1beta1.ListenerStatus, 0, len(gatewayStatus.ListenerStatuses)) // FIXME(pleshakov) Maintain the order from the Gateway resource + // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/689 names := make([]string, 0, len(gatewayStatus.ListenerStatuses)) for name := range gatewayStatus.ListenerStatuses { names = append(names, name) @@ -34,7 +32,9 @@ func prepareGatewayStatus( Name: v1beta1.SectionName(name), SupportedKinds: []v1beta1.RouteGroupKind{ { - Kind: "HTTPRoute", // FIXME(pleshakov) Set it based on the listener + // FIXME(pleshakov) Set it based on the listener + // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/690 + Kind: "HTTPRoute", }, }, AttachedRoutes: s.AttachedRoutes, diff --git a/internal/status/httproute.go b/internal/status/httproute.go index 7c22e63f55..f0e6a63c52 100644 --- a/internal/status/httproute.go +++ b/internal/status/httproute.go @@ -8,9 +8,6 @@ import ( ) // prepareHTTPRouteStatus prepares the status for an HTTPRoute resource. -// FIXME(pleshakov): Be compliant with in the Gateway API. -// Currently, we only support simple attached/not attached status per each parentRef. -// Extend support to cover more cases. func prepareHTTPRouteStatus( status state.HTTPRouteStatus, gatewayCtlrName string, diff --git a/internal/status/updater.go b/internal/status/updater.go index 88d1ebd211..a5323df070 100644 --- a/internal/status/updater.go +++ b/internal/status/updater.go @@ -44,10 +44,8 @@ type UpdaterConfig struct { // // (1) It doesn't understand the leader election. Only the leader must report the statuses of the resources. Otherwise, // multiple replicas will step on each other when trying to report statuses for the same resources. -// FIXME(pleshakov): address limitation (1) // // (2) It is not smart. It will update the status of a resource (make an API call) even if it hasn't changed. -// FIXME(pleshakov) address limitation (2) // // (3) It is synchronous, which means the status reporter can slow down the event loop. // Consider the following cases: @@ -55,26 +53,23 @@ type UpdaterConfig struct { // status API calls sequentially will take time. // (b) k8s API can become slow or even timeout. This will increase every update status API call. // Making updaterImpl asynchronous will prevent it from adding variable delays to the event loop. -// FIXME(pleshakov) address limitation (3) // // (4) It doesn't retry on failures. This means there is a chance that some resources will not have up-to-do statuses. // Statuses are important part of the Gateway API, so we need to ensure that the Gateway always keep the resources // statuses up-to-date. -// FIXME(pleshakov): address limitation (4) // // (5) It doesn't clear the statuses of a resources that are no longer handled by the Gateway. For example, if // an HTTPRoute resource no longer has the parentRef to the Gateway resources, the Gateway must update the status // of the resource to remove the status about the removed parentRef. -// FIXME(pleshakov): address limitation (5) // // (6) If another controllers changes the status of the Gateway/HTTPRoute resource so that the information set by our // Gateway is removed, our Gateway will not restore the status until the EventLoop invokes the StatusUpdater as a // result of processing some other new change to a resource(s). -// FIXME(pleshakov): Figure out if this is something that needs to be addressed. +// FIXME(pleshakov): Make updater production ready +// https://github.com/nginxinc/nginx-kubernetes-gateway/issues/691 -// (7) To support new resources, updaterImpl needs to be modified. Consider making updaterImpl extendable, so that it +// To support new resources, updaterImpl needs to be modified. Consider making updaterImpl extendable, so that it // goes along the Open-closed principle. -// FIXME(pleshakov): address limitation (7) type updaterImpl struct { cfg UpdaterConfig } @@ -88,7 +83,7 @@ func NewUpdater(cfg UpdaterConfig) Updater { func (upd *updaterImpl) Update(ctx context.Context, statuses state.Statuses) { // FIXME(pleshakov) Merge the new Conditions in the status with the existing Conditions - // FIXME(pleshakov) Skip the status update (API call) if the status hasn't changed. + // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/558 if upd.cfg.UpdateGatewayClassStatus && statuses.GatewayClassStatus != nil { upd.update( @@ -135,8 +130,6 @@ func (upd *updaterImpl) update( statusSetter func(client.Object), ) { // The function handles errors by reporting them in the logs. - // FIXME(pleshakov): figure out appropriate log level for these errors. Perhaps 3? - // We need to get the latest version of the resource. // Otherwise, the Update status API call can fail. // Note: the default client uses a cache for reads, so we're not making an unnecessary API call here.