From 1f7774cc1023166a70172370494ef7182d2c0351 Mon Sep 17 00:00:00 2001 From: Raul Marrero Date: Fri, 15 May 2020 14:31:57 +0100 Subject: [PATCH] Add status to VirtualServer and VirtualServerRoute --- cmd/nginx-ingress/main.go | 6 +- deployments/common/vs-definition.yaml | 42 +++ deployments/common/vsr-definition.yaml | 44 +++ .../templates/controller-vs-definition.yaml | 42 +++ .../templates/controller-vsr-definition.yaml | 44 +++ deployments/helm-chart/templates/rbac.yaml | 7 + deployments/rbac/rbac.yaml | 7 + .../command-line-arguments.md | 6 +- .../reporting-resources-status.md | 95 +++++++ ...server-and-virtualserverroute-resources.md | 18 +- internal/k8s/controller.go | 135 +++++++++- internal/k8s/handlers.go | 6 +- internal/k8s/status.go | 254 +++++++++++++++++- internal/k8s/status_test.go | 248 +++++++++++++++++ pkg/apis/configuration/v1/types.go | 38 ++- .../configuration/v1/zz_generated.deepcopy.go | 60 +++++ .../v1/fake/fake_virtualserver.go | 12 + .../v1/fake/fake_virtualserverroute.go | 12 + .../typed/configuration/v1/virtualserver.go | 17 ++ .../configuration/v1/virtualserverroute.go | 17 ++ .../suite/test_v_s_route_upstream_options.py | 2 +- 21 files changed, 1084 insertions(+), 28 deletions(-) diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 64f79ef859..c34d09e801 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -95,14 +95,14 @@ var ( (default for NGINX "nginx.transportserver.tmpl"; default for NGINX Plus "nginx-plus.transportserver.tmpl")`) externalService = flag.String("external-service", "", - `Specifies the name of the service with the type LoadBalancer through which the Ingress controller pods are exposed externally. -The external address of the service is used when reporting the status of Ingress resources. Requires -report-ingress-status.`) + `Specifies the name of the service with the type LoadBalancer through which the Ingress controller pods are exposed externally. + The external address of the service is used when reporting the status of Ingress, VirtualServer and VirtualServerRoute resources. For Ingress resources only: Requires -report-ingress-status.`) reportIngressStatus = flag.Bool("report-ingress-status", false, "Update the address field in the status of Ingresses resources. Requires the -external-service flag, or the 'external-status-address' key in the ConfigMap.") leaderElectionEnabled = flag.Bool("enable-leader-election", false, - "Enable Leader election to avoid multiple replicas of the controller reporting the status of Ingress resources -- only one replica will report status. See -report-ingress-status flag.") + "Enable Leader election to avoid multiple replicas of the controller reporting the status of Ingress, VirtualServer and VirtualServerRoute resources -- only one replica will report status. See -report-ingress-status flag.") leaderElectionLockName = flag.String("leader-election-lock-name", "nginx-ingress-leader-election", `Specifies the name of the ConfigMap, within the same namespace as the controller, used as the lock for leader election. Requires -enable-leader-election.`) diff --git a/deployments/common/vs-definition.yaml b/deployments/common/vs-definition.yaml index eb87b672fe..593767d84b 100644 --- a/deployments/common/vs-definition.yaml +++ b/deployments/common/vs-definition.yaml @@ -9,6 +9,8 @@ spec: served: true storage: true scope: Namespaced + subresources: + status: {} names: kind: VirtualServer plural: virtualservers @@ -16,6 +18,24 @@ spec: shortNames: - vs preserveUnknownFields: false + additionalPrinterColumns: + - name: State + type: string + description: Current state of the VirtualServer. If the resource has a valid status, + it means it has been validated and accepted by the Ingress Controller. + JSONPath: .status.state + - name: Host + type: string + JSONPath: .spec.host + - name: IP + type: string + JSONPath: .status.externalEndpoints[*].ip + - name: Ports + type: string + JSONPath: .status.externalEndpoints[*].ports + - name: Age + type: date + JSONPath: .metadata.creationTimestamp validation: openAPIV3Schema: description: VirtualServer defines the VirtualServer resource. @@ -375,3 +395,25 @@ spec: properties: enable: type: boolean + status: + description: VirtualServerStatus defines the status for the VirtualServer + resource. + type: object + properties: + externalEndpoints: + type: array + items: + description: ExternalEndpoint defines the IP and ports used to connect + to this resource. + type: object + properties: + ip: + type: string + ports: + type: string + message: + type: string + reason: + type: string + state: + type: string diff --git a/deployments/common/vsr-definition.yaml b/deployments/common/vsr-definition.yaml index 108b8fe9e5..e960db4363 100644 --- a/deployments/common/vsr-definition.yaml +++ b/deployments/common/vsr-definition.yaml @@ -9,6 +9,8 @@ spec: served: true storage: true scope: Namespaced + subresources: + status: {} names: kind: VirtualServerRoute plural: virtualserverroutes @@ -16,6 +18,24 @@ spec: shortNames: - vsr preserveUnknownFields: false + additionalPrinterColumns: + - name: State + type: string + description: Current state of the VirtualServerRoute. If the resource has a valid + status, it means it has been validated and accepted by the Ingress Controller. + JSONPath: .status.state + - name: Host + type: string + JSONPath: .spec.host + - name: IP + type: string + JSONPath: .status.externalEndpoints[*].ip + - name: Ports + type: string + JSONPath: .status.externalEndpoints[*].ports + - name: Age + type: date + JSONPath: .metadata.creationTimestamp validation: openAPIV3Schema: type: object @@ -357,3 +377,27 @@ spec: properties: enable: type: boolean + status: + description: VirtualServerRouteStatus defines the status for the VirtualServerRoute + resource. + type: object + properties: + externalEndpoints: + type: array + items: + description: ExternalEndpoint defines the IP and ports used to connect + to this resource. + type: object + properties: + ip: + type: string + ports: + type: string + message: + type: string + reason: + type: string + referencedBy: + type: string + state: + type: string diff --git a/deployments/helm-chart/templates/controller-vs-definition.yaml b/deployments/helm-chart/templates/controller-vs-definition.yaml index e1ef051364..8936296b76 100644 --- a/deployments/helm-chart/templates/controller-vs-definition.yaml +++ b/deployments/helm-chart/templates/controller-vs-definition.yaml @@ -12,6 +12,8 @@ spec: served: true storage: true scope: Namespaced + subresources: + status: {} names: kind: VirtualServer plural: virtualservers @@ -19,6 +21,24 @@ spec: shortNames: - vs preserveUnknownFields: false + additionalPrinterColumns: + - name: State + type: string + description: Current state of the VirtualServer. If the resource has a valid status, + it means it has been validated and accepted by the Ingress Controller. + JSONPath: .status.state + - name: Host + type: string + JSONPath: .spec.host + - name: IP + type: string + JSONPath: .status.externalEndpoints[*].ip + - name: Ports + type: string + JSONPath: .status.externalEndpoints[*].ports + - name: Age + type: date + JSONPath: .metadata.creationTimestamp validation: openAPIV3Schema: description: VirtualServer defines the VirtualServer resource. @@ -378,4 +398,26 @@ spec: properties: enable: type: boolean + status: + description: VirtualServerStatus defines the status for the VirtualServer + resource. + type: object + properties: + externalEndpoints: + type: array + items: + description: ExternalEndpoint defines the IP and ports used to connect + to this resource. + type: object + properties: + ip: + type: string + ports: + type: string + message: + type: string + reason: + type: string + state: + type: string {{- end }} diff --git a/deployments/helm-chart/templates/controller-vsr-definition.yaml b/deployments/helm-chart/templates/controller-vsr-definition.yaml index 202268a028..3da6d71648 100644 --- a/deployments/helm-chart/templates/controller-vsr-definition.yaml +++ b/deployments/helm-chart/templates/controller-vsr-definition.yaml @@ -12,6 +12,8 @@ spec: served: true storage: true scope: Namespaced + subresources: + status: {} names: kind: VirtualServerRoute plural: virtualserverroutes @@ -19,6 +21,24 @@ spec: shortNames: - vsr preserveUnknownFields: false + additionalPrinterColumns: + - name: State + type: string + description: Current state of the VirtualServerRoute. If the resource has a valid + status, it means it has been validated and accepted by the Ingress Controller. + JSONPath: .status.state + - name: Host + type: string + JSONPath: .spec.host + - name: IP + type: string + JSONPath: .status.externalEndpoints[*].ip + - name: Ports + type: string + JSONPath: .status.externalEndpoints[*].ports + - name: Age + type: date + JSONPath: .metadata.creationTimestamp validation: openAPIV3Schema: type: object @@ -360,4 +380,28 @@ spec: properties: enable: type: boolean + status: + description: VirtualServerRouteStatus defines the status for the VirtualServerRoute + resource. + type: object + properties: + externalEndpoints: + type: array + items: + description: ExternalEndpoint defines the IP and ports used to connect + to this resource. + type: object + properties: + ip: + type: string + ports: + type: string + message: + type: string + reason: + type: string + referencedBy: + type: string + state: + type: string {{- end }} diff --git a/deployments/helm-chart/templates/rbac.yaml b/deployments/helm-chart/templates/rbac.yaml index 32ae2261e5..2df801dd0c 100644 --- a/deployments/helm-chart/templates/rbac.yaml +++ b/deployments/helm-chart/templates/rbac.yaml @@ -76,6 +76,13 @@ rules: - list - watch - get +- apiGroups: + - k8s.nginx.org + resources: + - virtualservers/status + - virtualserverroutes/status + verbs: + - update {{- end }} --- kind: ClusterRoleBinding diff --git a/deployments/rbac/rbac.yaml b/deployments/rbac/rbac.yaml index 92cbd0b1ee..c0ac80583f 100644 --- a/deployments/rbac/rbac.yaml +++ b/deployments/rbac/rbac.yaml @@ -69,6 +69,13 @@ rules: - list - watch - get +- apiGroups: + - k8s.nginx.org + resources: + - virtualservers/status + - virtualserverroutes/status + verbs: + - update --- kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1beta1 diff --git a/docs-web/configuration/global-configuration/command-line-arguments.md b/docs-web/configuration/global-configuration/command-line-arguments.md index 129e7f71d5..13e0f80b06 100644 --- a/docs-web/configuration/global-configuration/command-line-arguments.md +++ b/docs-web/configuration/global-configuration/command-line-arguments.md @@ -31,7 +31,7 @@ Below we describe the available command-line arguments: .. option:: -enable-leader-election - Enables Leader election to avoid multiple replicas of the controller reporting the status of Ingress resources -- only one replica will report status. + Enables Leader election to avoid multiple replicas of the controller reporting the status of Ingress, VirtualServer and VirtualServerRoute resources -- only one replica will report status. See :option:`-report-ingress-status` flag. @@ -43,9 +43,9 @@ Below we describe the available command-line arguments: .. option:: -external-service - Specifies the name of the service with the type LoadBalancer through which the Ingress controller pods are exposed externally. The external address of the service is used when reporting the status of Ingress resources. + Specifies the name of the service with the type LoadBalancer through which the Ingress controller pods are exposed externally. The external address of the service is used when reporting the status of Ingress, VirtualServer and VirtualServerRoute resources. - Requires :option:`-report-ingress-status`. + For Ingress resources only: Requires :option:`-report-ingress-status`. .. option:: -global-configuration diff --git a/docs-web/configuration/global-configuration/reporting-resources-status.md b/docs-web/configuration/global-configuration/reporting-resources-status.md index 0708faed86..5ef2b892fb 100644 --- a/docs-web/configuration/global-configuration/reporting-resources-status.md +++ b/docs-web/configuration/global-configuration/reporting-resources-status.md @@ -1,5 +1,7 @@ # Reporting Resources Status +## Ingress Resources + An Ingress resource can have a status that includes the address (an IP address or a DNS name), through which the hosts of that Ingress resource are publicly accessible. You can see the address in the output of the `kubectl get ingress` command, in the ADDRESS column, as shown below: @@ -22,3 +24,96 @@ to ensure that only one replica updates an Ingress status. See the docs about [ConfigMap keys](/nginx-ingress-controller/configuration/global-configuration/configmap-resource) and [Command-line arguments](/nginx-ingress-controller/configuration/global-configuration/command-line-arguments). Notes: The Ingress controller does not clear the status of Ingress resources when it is being shut down. + +## VirtualServer and VirtualServerRoute Resources + +A VirtualServer or VirtualServerRoute resource includes the status field with information about the state of the resource and the IP address, through which the hosts of that resource are publicly accessible. +You can see the status in the output of the `kubectl get virtualservers` or `kubectl get virtualserverroutes` commands as shown below: + +``` +$ kubectl get virtualservers + NAME STATE HOST IP PORTS AGE + cafe Valid cafe.example.com 12.13.23.123 [80,443] 34s +``` + +> Note: If there are multiple addresses, only the first one is shown. + +In order to see additional addresses or extra information about the `Status` of the resource, use the following command: +``` +$ kubectl describe virtualserver +. . . +Status: + External Endpoints: + Ip: 12.13.23.123 + Ports: [80,443] + Message: Configuration for cafe/cafe was added or updated + Reason: AddedOrUpdated + State: Valid +``` + +### Status Specification +The following fields are reported in both VirtualServer and VirtualServerRoute status: + +```eval_rst +.. list-table:: + :header-rows: 1 + + * - Field + - Description + - Type + * - ``State`` + - Current state of the resource. Can be ``Valid``, ``Warning`` an ``Invalid``. For more information, refer to the ``message`` field. + - ``string`` + * - ``Reason`` + - The reason of the last update. + - ``string`` + * - ``Message`` + - Additional information about the state. + - ``string`` + * - ``ExternalEndpoints`` + - A list of external endpoints for which the hosts of the resource are publicly accessible. + - `[]externalEndpoint <#externalendpoint>`_ +``` + +The following field is reported in the VirtualServerRoute status only: + +```eval_rst +.. list-table:: + :header-rows: 1 + + * - Field + - Description + - Type + * - ``ReferencedBy`` + - The VirtualServer that references this VirtualServerRoute. Format is ``namespace/name`` + - ``string`` +``` + +### ExternalEndpoint +```eval_rst +.. list-table:: + :header-rows: 1 + + * - Field + - Description + - Type + * - ``IP`` + - The external IP address. + - ``string`` + * - ``Ports`` + - A list of external ports. + - ``string`` +``` + +The Ingress controller must be configured to report a VirtualServer or VirtualServerRoute status: + +1. If you want the Ingress controller to report the `externalEndpoints`, define a source for an external address (Note: the rest of the fields will be reported without the external address configured). This can be either of: + 1. A user defined address, specified in the `external-status-address` ConfigMap key. + 2. A Service of the type LoadBalancer configured with an external IP or address and specified by the `-external-service` command-line flag. +1. If you're running multiple replicas of the Ingress controller, enable leader election with the `-enable-leader-election` flag +to ensure that only one replica updates an Ingress status. +1. By default, the Ingress controller will use a ConfigMap with the name `nginx-ingress-leader-election` as the lock. This can be customised via the `-leader-election-lock-name` flag. + +See the docs about [ConfigMap keys](/nginx-ingress-controller/configuration/global-configuration/configmap-resource) and [Command-line arguments](/nginx-ingress-controller/configuration/global-configuration/command-line-arguments). + +Notes: The Ingress controller does not clear the status of VirtualServer and VirtualServerRoute resources when it is being shut down. \ No newline at end of file diff --git a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md index 851761d8f7..385c4ffb91 100644 --- a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md @@ -1099,8 +1099,8 @@ virtualserver.k8s.nginx.org "cafe" created You can get the resource by running: ``` $ kubectl get virtualserver cafe -NAME AGE -cafe 3m +NAME STATE HOST IP PORTS AGE +cafe Valid cafe.example.com 12.13.23.123 [80,443] 3m ``` In the kubectl get and similar commands, you can also use the short name `vs` instead of `virtualserver`. @@ -1158,6 +1158,20 @@ Events: ``` Note how the events section includes a Warning event with the Rejected reason. +Additionally, this information is also available in the `status` field of the VirtualServer resource. Note the Status section of the VirtualServer: + +``` +$ kubectl describe vs cafe +. . . +Status: + External Endpoints: + Ip: 12.13.23.123 + Ports: [80,443] + Message: VirtualServer default/cafe is invalid and was rejected: spec.upstreams[1].name: Duplicate value: "tea" + Reason: Rejected + State: Invalid +``` + The Ingress Controller validates VirtualServerRoute resources in a similar way. **Note**: If you make an existing resource invalid, the Ingress Controller will reject it and remove the corresponding configuration from NGINX. diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index d193d8a411..194b325802 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -185,6 +185,7 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc externalServiceName: input.ExternalServiceName, ingLister: &lbc.ingressLister, keyFunc: keyFunc, + confClient: input.ConfClient, } // create handlers for resources we care about @@ -547,6 +548,12 @@ func (lbc *LoadBalancerController) syncConfig(task task) { if lbc.areCustomResourcesEnabled { virtualServers := lbc.getVirtualServers() virtualServerExes = lbc.virtualServersToVirtualServerExes(virtualServers) + if lbc.reportVsVsrStatusEnabled() { + err = lbc.statusUpdater.UpdateVsVsrExternalEndpoints(virtualServers, lbc.getVirtualServerRoutes()) + if err != nil { + glog.V(3).Infof("error updating status on ConfigMap change: %v", err) + } + } } warnings, updateErr := lbc.configurator.UpdateConfig(cfgParams, ingExes, mergeableIngresses, virtualServerExes) @@ -586,27 +593,49 @@ func (lbc *LoadBalancerController) syncConfig(task task) { vsEventType := eventType vsEventTitle := eventTitle vsEventWarningMessage := eventWarningMessage + vsState := conf_v1.StateValid if messages, ok := warnings[vsEx.VirtualServer]; ok && updateErr == nil { vsEventType = api_v1.EventTypeWarning vsEventTitle = "UpdatedWithWarning" vsEventWarningMessage = fmt.Sprintf("with warning(s): %v", formatWarningMessages(messages)) + vsState = conf_v1.StateWarning } - lbc.recorder.Eventf(vsEx.VirtualServer, vsEventType, vsEventTitle, "Configuration for %v/%v was updated %s", - vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, vsEventWarningMessage) + msg := fmt.Sprintf("Configuration for %v/%v was updated %s", vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, vsEventWarningMessage) + lbc.recorder.Eventf(vsEx.VirtualServer, vsEventType, vsEventTitle, msg) + + if lbc.reportVsVsrStatusEnabled() { + err = lbc.statusUpdater.UpdateVirtualServerStatus(vsEx.VirtualServer, vsState, vsEventTitle, msg) + + if err != nil { + glog.Errorf("Error when updating the status for VirtualServer %v/%v: %v", vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, err) + } + } for _, vsr := range vsEx.VirtualServerRoutes { vsrEventType := eventType vsrEventTitle := eventTitle vsrEventWarningMessage := eventWarningMessage + vsrState := conf_v1.StateValid + if messages, ok := warnings[vsr]; ok && updateErr == nil { vsrEventType = api_v1.EventTypeWarning vsrEventTitle = "UpdatedWithWarning" vsrEventWarningMessage = fmt.Sprintf("with warning(s): %v", formatWarningMessages(messages)) + vsrState = conf_v1.StateWarning + } + + msg := fmt.Sprintf("Configuration for %v/%v was updated %s", vsr.Namespace, vsr.Name, vsrEventWarningMessage) + lbc.recorder.Eventf(vsr, vsrEventType, vsrEventTitle, msg) + + if lbc.reportVsVsrStatusEnabled() { + err = lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, vsrState, vsrEventTitle, vsrEventWarningMessage) + + if err != nil { + glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) + } } - lbc.recorder.Eventf(vsr, vsrEventType, vsrEventTitle, "Configuration for %v/%v was updated %s", - vsr.Namespace, vsr.Name, vsrEventWarningMessage) } } } @@ -863,7 +892,18 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { if err != nil { glog.Errorf("Error when deleting configuration for %v: %v", key, err) } - lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "Rejected", "VirtualServer %v is invalid and was rejected: %v", key, validationErr) + reason := "Rejected" + msg := fmt.Sprintf("VirtualServer %v is invalid and was rejected: %v", key, validationErr) + + lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, reason, msg) + if lbc.reportVsVsrStatusEnabled() { + err = lbc.statusUpdater.UpdateVirtualServerStatus(vs, conf_v1.StateInvalid, reason, msg) + + if err != nil { + glog.Errorf("Error when updating the status for VirtualServer %v/%v: %v", vs.Namespace, vs.Name, err) + } + } + // TO-DO: emit events for referenced VirtualServerRoutes return } @@ -882,11 +922,13 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { eventTitle := "AddedOrUpdated" eventType := api_v1.EventTypeNormal eventWarningMessage := "" + state := conf_v1.StateValid if addErr != nil { eventTitle = "AddedOrUpdatedWithError" eventType = api_v1.EventTypeWarning eventWarningMessage = fmt.Sprintf("but was not applied: %v", addErr) + state = conf_v1.StateInvalid } vsEventType := eventType @@ -897,21 +939,43 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { vsEventType = api_v1.EventTypeWarning vsEventTitle = "AddedOrUpdatedWithWarning" vsEventWarningMessage = fmt.Sprintf("with warning(s): %v", formatWarningMessages(messages)) + state = conf_v1.StateWarning } - lbc.recorder.Eventf(vs, vsEventType, vsEventTitle, "Configuration for %v was added or updated %s", key, vsEventWarningMessage) + msg := fmt.Sprintf("Configuration for %v was added or updated %s", key, vsEventWarningMessage) + lbc.recorder.Eventf(vs, vsEventType, vsEventTitle, msg) + + if lbc.reportVsVsrStatusEnabled() { + err = lbc.statusUpdater.UpdateVirtualServerStatus(vs, state, vsEventTitle, msg) + + if err != nil { + glog.Errorf("Error when updating the status for VirtualServer %v/%v: %v", vs.Namespace, vs.Name, err) + } + } for _, vsr := range vsEx.VirtualServerRoutes { vsrEventType := eventType vsrEventTitle := eventTitle vsrEventWarningMessage := eventWarningMessage + state := conf_v1.StateValid if messages, ok := warnings[vsr]; ok && addErr == nil { vsrEventType = api_v1.EventTypeWarning vsrEventTitle = "AddedOrUpdatedWithWarning" vsrEventWarningMessage = fmt.Sprintf("with warning(s): %v", formatWarningMessages(messages)) + state = conf_v1.StateWarning + } + msg := fmt.Sprintf("Configuration for %v/%v was added or updated %s", vsr.Namespace, vsr.Name, vsrEventWarningMessage) + lbc.recorder.Eventf(vsr, vsrEventType, vsrEventTitle, msg) + + if lbc.reportVsVsrStatusEnabled() { + virtualServersForVSR := findVirtualServersForVirtualServerRoute(lbc.getVirtualServers(), vsr) + err = lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, state, vsrEventTitle, msg, virtualServersForVSR) + + if err != nil { + glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) + } } - lbc.recorder.Eventf(vsr, vsrEventType, vsrEventTitle, "Configuration for %v/%v was added or updated %s", vsr.Namespace, vsr.Name, vsrEventWarningMessage) } } @@ -938,15 +1002,30 @@ func (lbc *LoadBalancerController) syncVirtualServerRoute(task task) { validationErr := validation.ValidateVirtualServerRoute(vsr, lbc.isNginxPlus) if validationErr != nil { - lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, "Rejected", "VirtualServerRoute %s is invalid and was rejected: %v", key, validationErr) + reason := "Rejected" + msg := fmt.Sprintf("VirtualServerRoute %s is invalid and was rejected: %v", key, validationErr) + lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) + if lbc.reportVsVsrStatusEnabled() { + err = lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) + if err != nil { + glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) + } + } } - vsCount := lbc.enqueueVirtualServersForVirtualServerRouteKey(key) if vsCount == 0 { - lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, "NoVirtualServersFound", "No VirtualServer references VirtualServerRoute %s", key) - } + reason := "NoVirtualServersFound" + msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %s", key) + lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) + if lbc.reportVsVsrStatusEnabled() { + err = lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) + if err != nil { + glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) + } + } + } } func (lbc *LoadBalancerController) syncIngMinion(task task) { @@ -1104,6 +1183,13 @@ func (lbc *LoadBalancerController) syncExternalService(task task) { glog.Errorf("error updating ingress status in syncExternalService: %v", err) } } + + if lbc.areCustomResourcesEnabled && lbc.reportVsVsrStatusEnabled() { + err = lbc.statusUpdater.UpdateVsVsrExternalEndpoints(lbc.getVirtualServers(), lbc.getVirtualServerRoutes()) + if err != nil { + glog.V(3).Infof("error updating VirtualServer/VirtualServerRoute status in syncExternalService: %v", err) + } + } } // IsExternalServiceForStatus matches the service specified by the external-service arg @@ -1111,7 +1197,7 @@ func (lbc *LoadBalancerController) IsExternalServiceForStatus(svc *api_v1.Servic return lbc.statusUpdater.namespace == svc.Namespace && lbc.statusUpdater.externalServiceName == svc.Name } -// reportStatusEnabled determines if we should attempt to report status +// reportStatusEnabled determines if we should attempt to report status for Ingress resources. func (lbc *LoadBalancerController) reportStatusEnabled() bool { if lbc.reportIngressStatus { if lbc.isLeaderElectionEnabled { @@ -1122,6 +1208,15 @@ func (lbc *LoadBalancerController) reportStatusEnabled() bool { return false } +// reportVsVsrStatusEnabled determines if we should attempt to report status for VirtualServers and VirtualServerRoutes. +func (lbc *LoadBalancerController) reportVsVsrStatusEnabled() bool { + if lbc.isLeaderElectionEnabled { + return lbc.leaderElector != nil && lbc.leaderElector.IsLeader() + } + + return true +} + func (lbc *LoadBalancerController) syncSecret(task task) { key := task.Key obj, secrExists, err := lbc.secretLister.Store.GetByKey(key) @@ -1182,9 +1277,11 @@ func (lbc *LoadBalancerController) handleRegularSecretDeletion(key string, ings eventType := api_v1.EventTypeWarning title := "Missing Secret" message := fmt.Sprintf("Secret %v was removed", key) + state := conf_v1.StateInvalid lbc.emitEventForIngresses(eventType, title, message, ings) lbc.emitEventForVirtualServers(eventType, title, message, virtualServers) + lbc.updateStatusForVirtualServers(state, title, message, virtualServers) regular, mergeable := lbc.createIngresses(ings) @@ -1200,10 +1297,12 @@ func (lbc *LoadBalancerController) handleRegularSecretDeletion(key string, ings eventType = api_v1.EventTypeWarning title = "UpdatedWithError" message = fmt.Sprintf("Configuration was updated due to removed secret %v, but not applied: %v", key, err) + state = conf_v1.StateInvalid } lbc.emitEventForIngresses(eventType, title, message, ings) lbc.emitEventForVirtualServers(eventType, title, message, virtualServers) + lbc.updateStatusForVirtualServers(state, title, message, virtualServers) } func (lbc *LoadBalancerController) handleSecretUpdate(secret *api_v1.Secret, ings []extensions.Ingress, virtualServers []*conf_v1.VirtualServer) { @@ -1224,6 +1323,7 @@ func (lbc *LoadBalancerController) handleSecretUpdate(secret *api_v1.Secret, ing eventType := api_v1.EventTypeNormal title := "Updated" message := fmt.Sprintf("Configuration was updated due to updated secret %v", secretNsName) + state := conf_v1.StateValid // we can safely ignore the error because the secret is valid in this function kind, _ := GetSecretKind(secret) @@ -1243,11 +1343,13 @@ func (lbc *LoadBalancerController) handleSecretUpdate(secret *api_v1.Secret, ing eventType = api_v1.EventTypeWarning title = "UpdatedWithError" message = fmt.Sprintf("Configuration was updated due to updated secret %v, but not applied: %v", secretNsName, err) + state = conf_v1.StateInvalid } } lbc.emitEventForIngresses(eventType, title, message, ings) lbc.emitEventForVirtualServers(eventType, title, message, virtualServers) + lbc.updateStatusForVirtualServers(state, title, message, virtualServers) } func (lbc *LoadBalancerController) handleSpecialSecretUpdate(secret *api_v1.Secret) { @@ -1298,6 +1400,15 @@ func (lbc *LoadBalancerController) emitEventForVirtualServers(eventType string, } } +func (lbc *LoadBalancerController) updateStatusForVirtualServers(state string, reason string, message string, virtualServers []*conf_v1.VirtualServer) { + for _, vs := range virtualServers { + err := lbc.statusUpdater.UpdateVirtualServerStatus(vs, state, reason, message) + if err != nil { + glog.Errorf("Error when updating the status for VirtualServer %v/%v: %v", vs.Namespace, vs.Name, err) + } + } +} + func (lbc *LoadBalancerController) createIngresses(ings []extensions.Ingress) (regular []configs.IngressEx, mergeable []configs.MergeableIngresses) { for i := range ings { if isMaster(&ings[i]) { diff --git a/internal/k8s/handlers.go b/internal/k8s/handlers.go index 42ad0c74b3..6b62d9f57f 100644 --- a/internal/k8s/handlers.go +++ b/internal/k8s/handlers.go @@ -338,7 +338,8 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven }, UpdateFunc: func(old, cur interface{}) { curVs := cur.(*conf_v1.VirtualServer) - if !reflect.DeepEqual(old, cur) { + oldVs := old.(*conf_v1.VirtualServer) + if !reflect.DeepEqual(oldVs.Spec, curVs.Spec) { glog.V(3).Infof("VirtualServer %v changed, syncing", curVs.Name) lbc.AddSyncQueue(curVs) } @@ -372,7 +373,8 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc }, UpdateFunc: func(old, cur interface{}) { curVsr := cur.(*conf_v1.VirtualServerRoute) - if !reflect.DeepEqual(old, cur) { + oldVsr := old.(*conf_v1.VirtualServerRoute) + if !reflect.DeepEqual(oldVsr.Spec, curVsr.Spec) { glog.V(3).Infof("VirtualServerRoute %v changed, syncing", curVsr.Name) lbc.AddSyncQueue(curVsr) } diff --git a/internal/k8s/status.go b/internal/k8s/status.go index c4700aa431..debe9f279e 100644 --- a/internal/k8s/status.go +++ b/internal/k8s/status.go @@ -5,18 +5,24 @@ import ( "fmt" "net" "reflect" + "strconv" + "strings" "github.com/golang/glog" "github.com/nginxinc/kubernetes-ingress/internal/configs" + conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" + v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" + k8s_nginx "github.com/nginxinc/kubernetes-ingress/pkg/client/clientset/versioned" api_v1 "k8s.io/api/core/v1" "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes" extensionsv1beta1 "k8s.io/client-go/kubernetes/typed/extensions/v1beta1" ) -// statusUpdater reports Ingress status information via the kubernetes -// API, primarily the IP or host of the LoadBalancer Service exposing the +// statusUpdater reports Ingress, VirtualServer and VirtualServerRoute status information via the kubernetes +// API. For external information, it primarily reports the IP or host of the LoadBalancer Service exposing the // Ingress Controller, or an external IP specified in the ConfigMap. type statusUpdater struct { client kubernetes.Interface @@ -24,9 +30,12 @@ type statusUpdater struct { externalServiceName string externalStatusAddress string externalServiceAddresses []string + externalServicePorts string + externalEndpoints []v1.ExternalEndpoint status []api_v1.LoadBalancerIngress keyFunc func(obj interface{}) (string, error) ingLister *storeToIngressLister + confClient k8s_nginx.Interface } // UpdateManagedAndMergeableIngresses handles the full return format of LoadBalancerController.getManagedIngresses @@ -158,6 +167,42 @@ func (su *statusUpdater) saveStatus(ips []string) { su.status = statusIngs } +var intPorts = [2]int32{80, 443} +var stringPorts = [2]string{"http", "https"} + +func isRequiredPort(port intstr.IntOrString) bool { + if port.Type == intstr.Int { + for _, p := range intPorts { + if p == port.IntVal { + return true + } + } + } else if port.Type == intstr.String { + for _, p := range stringPorts { + if p == port.StrVal { + return true + } + } + } + + return false +} + +func getExternalServicePorts(svc *api_v1.Service) string { + var ports []string + if svc == nil { + return "" + } + + for _, port := range svc.Spec.Ports { + if isRequiredPort(port.TargetPort) { + ports = append(ports, strconv.Itoa(int(port.Port))) + } + } + + return fmt.Sprintf("[%v]", strings.Join(ports, ",")) +} + func getExternalServiceAddress(svc *api_v1.Service) []string { addresses := []string{} if svc == nil { @@ -190,12 +235,14 @@ func (su *statusUpdater) SaveStatusFromExternalStatus(externalStatusAddress stri // external service if it exists if len(su.externalServiceAddresses) > 0 { su.saveStatus(su.externalServiceAddresses) + su.externalEndpoints = su.generateExternalEndpointsFromStatus(su.status) return } } ips := []string{} ips = append(ips, su.externalStatusAddress) su.saveStatus(ips) + su.externalEndpoints = su.generateExternalEndpointsFromStatus(su.status) } // ClearStatusFromExternalService clears the saved status from the External Service @@ -208,9 +255,210 @@ func (su *statusUpdater) ClearStatusFromExternalService() { func (su *statusUpdater) SaveStatusFromExternalService(svc *api_v1.Service) { ips := getExternalServiceAddress(svc) su.externalServiceAddresses = ips + ports := getExternalServicePorts(svc) + su.externalServicePorts = ports if su.externalStatusAddress != "" { - glog.V(3).Info("skipping external service address - external-status-address is set and takes precedence") + glog.V(3).Info("skipping external service address/ports - external-status-address is set and takes precedence") return } su.saveStatus(ips) + su.externalEndpoints = su.generateExternalEndpointsFromStatus(su.status) +} + +func (su *statusUpdater) retryUpdateVirtualServerStatus(vsCopy *conf_v1.VirtualServer) error { + vs, err := su.confClient.K8sV1().VirtualServers(vsCopy.Namespace).Get(context.TODO(), vsCopy.Name, metav1.GetOptions{}) + if err != nil { + return err + } + + vs.Status = vsCopy.Status + _, err = su.confClient.K8sV1().VirtualServers(vs.Namespace).UpdateStatus(context.TODO(), vs, metav1.UpdateOptions{}) + if err != nil { + return err + } + + return nil +} + +func (su *statusUpdater) retryUpdateVirtualServerRouteStatus(vsrCopy *conf_v1.VirtualServerRoute) error { + vsr, err := su.confClient.K8sV1().VirtualServerRoutes(vsrCopy.Namespace).Get(context.TODO(), vsrCopy.Name, metav1.GetOptions{}) + if err != nil { + return err + } + + vsr.Status = vsrCopy.Status + _, err = su.confClient.K8sV1().VirtualServerRoutes(vsr.Namespace).UpdateStatus(context.TODO(), vsr, metav1.UpdateOptions{}) + if err != nil { + return err + } + + return nil +} + +func hasVsStatusChanged(vs *conf_v1.VirtualServer, state string, reason string, message string) bool { + if vs.Status.State != state { + return true + } + + if vs.Status.Reason != reason { + return true + } + + if vs.Status.Message != message { + return true + } + + return false +} + +// UpdateVirtualServerStatus updates the status of a VirtualServer. +func (su *statusUpdater) UpdateVirtualServerStatus(vs *conf_v1.VirtualServer, state string, reason string, message string) error { + if !hasVsStatusChanged(vs, state, reason, message) { + return nil + } + + vsCopy := vs.DeepCopy() + vsCopy.Status.State = state + vsCopy.Status.Reason = reason + vsCopy.Status.Message = message + vsCopy.Status.ExternalEndpoints = su.externalEndpoints + + _, err := su.confClient.K8sV1().VirtualServers(vsCopy.Namespace).UpdateStatus(context.TODO(), vsCopy, metav1.UpdateOptions{}) + if err != nil { + glog.V(3).Infof("error setting VirtualServer %v/%v status, retrying: %v", vsCopy.Namespace, vsCopy.Name, err) + return su.retryUpdateVirtualServerStatus(vsCopy) + } + return err +} + +func hasVsrStatusChanged(vsr *conf_v1.VirtualServerRoute, state string, reason string, message string, referencedByString string) bool { + if vsr.Status.State != state { + return true + } + + if vsr.Status.Reason != reason { + return true + } + + if vsr.Status.Message != message { + return true + } + + if referencedByString != "" && vsr.Status.ReferencedBy != referencedByString { + return true + } + + return false +} + +// UpdateVirtualServerRouteStatusWithReferencedBy updates the status of a VirtualServerRoute, including the referencedBy field. +func (su *statusUpdater) UpdateVirtualServerRouteStatusWithReferencedBy(vsr *conf_v1.VirtualServerRoute, state string, reason string, message string, referencedBy []*v1.VirtualServer) error { + var referencedByString string + if len(referencedBy) != 0 { + vs := referencedBy[0] + referencedByString = fmt.Sprintf("%v/%v", vs.Namespace, vs.Name) + } + + vsrCopy := vsr.DeepCopy() + vsrCopy.Status.State = state + vsrCopy.Status.Reason = reason + vsrCopy.Status.Message = message + vsrCopy.Status.ReferencedBy = referencedByString + vsrCopy.Status.ExternalEndpoints = su.externalEndpoints + + _, err := su.confClient.K8sV1().VirtualServerRoutes(vsrCopy.Namespace).UpdateStatus(context.TODO(), vsrCopy, metav1.UpdateOptions{}) + if err != nil { + glog.V(3).Infof("error setting VirtualServerRoute %v/%v status, retrying: %v", vsrCopy.Namespace, vsrCopy.Name, err) + return su.retryUpdateVirtualServerRouteStatus(vsrCopy) + } + return err +} + +// UpdateVirtualServerRouteStatus updates the status of a VirtualServerRoute. +// This method does not clear or update the referencedBy field of the status. +// If you need to update the referencedBy field, use UpdateVirtualServerRouteStatusWithReferencedBy instead. +func (su *statusUpdater) UpdateVirtualServerRouteStatus(vsr *conf_v1.VirtualServerRoute, state string, reason string, message string) error { + + if !hasVsrStatusChanged(vsr, state, reason, message, "") { + return nil + } + + vsrCopy := vsr.DeepCopy() + vsrCopy.Status.State = state + vsrCopy.Status.Reason = reason + vsrCopy.Status.Message = message + vsrCopy.Status.ExternalEndpoints = su.externalEndpoints + + _, err := su.confClient.K8sV1().VirtualServerRoutes(vsrCopy.Namespace).UpdateStatus(context.TODO(), vsrCopy, metav1.UpdateOptions{}) + if err != nil { + glog.V(3).Infof("error setting VirtualServerRoute %v/%v status, retrying: %v", vsrCopy.Namespace, vsrCopy.Name, err) + return su.retryUpdateVirtualServerRouteStatus(vsrCopy) + } + return err +} + +func (su *statusUpdater) updateVirtualServerExternalEndpoints(vs *conf_v1.VirtualServer) error { + vsCopy := vs.DeepCopy() + vsCopy.Status.ExternalEndpoints = su.externalEndpoints + + _, err := su.confClient.K8sV1().VirtualServers(vsCopy.Namespace).UpdateStatus(context.TODO(), vsCopy, metav1.UpdateOptions{}) + if err != nil { + glog.V(3).Infof("error setting VirtualServer %v/%v status, retrying: %v", vsCopy.Namespace, vsCopy.Name, err) + return su.retryUpdateVirtualServerStatus(vsCopy) + } + return err +} + +func (su *statusUpdater) updateVirtualServerRouteExternalEndpoints(vsr *conf_v1.VirtualServerRoute) error { + vsrCopy := vsr.DeepCopy() + vsrCopy.Status.ExternalEndpoints = su.externalEndpoints + + _, err := su.confClient.K8sV1().VirtualServerRoutes(vsrCopy.Namespace).UpdateStatus(context.TODO(), vsrCopy, metav1.UpdateOptions{}) + if err != nil { + glog.V(3).Infof("error setting VirtualServerRoute %v/%v status, retrying: %v", vsrCopy.Namespace, vsrCopy.Name, err) + return su.retryUpdateVirtualServerRouteStatus(vsrCopy) + } + return err +} + +// UpdateVsVsrExternalEndpoints updates all the external endpoints for the given VirtualServer and VirtualServerRoutes statuses. +func (su *statusUpdater) UpdateVsVsrExternalEndpoints(vss []*conf_v1.VirtualServer, vsrs []*conf_v1.VirtualServerRoute) error { + if len(vss) < 1 && len(vsrs) < 1 { + glog.V(3).Info("no VirtualServers or VirtualServerRoutes to update") + return nil + } + + var errorMsg string + for _, vs := range vss { + err := su.updateVirtualServerExternalEndpoints(vs) + if err != nil { + errorMsg = "not all VirtualServers updated" + } + } + + for _, vsr := range vsrs { + err := su.updateVirtualServerRouteExternalEndpoints(vsr) + if err != nil { + if errorMsg != "" { + errorMsg += ", " + } + errorMsg += "not all VirtualServerRoutes updated" + } + } + + if errorMsg != "" { + return fmt.Errorf(errorMsg) + } + + return nil +} + +func (su *statusUpdater) generateExternalEndpointsFromStatus(status []api_v1.LoadBalancerIngress) []conf_v1.ExternalEndpoint { + var externalEndpoints []conf_v1.ExternalEndpoint + for _, lb := range status { + endpoint := conf_v1.ExternalEndpoint{IP: lb.IP, Ports: su.externalServicePorts} + externalEndpoints = append(externalEndpoints, endpoint) + } + + return externalEndpoints } diff --git a/internal/k8s/status_test.go b/internal/k8s/status_test.go index 2b2d91b1f7..7396494d1b 100644 --- a/internal/k8s/status_test.go +++ b/internal/k8s/status_test.go @@ -2,12 +2,15 @@ package k8s import ( "context" + "reflect" "testing" + conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" v1 "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/cache" ) @@ -121,3 +124,248 @@ func checkStatus(expected string, actual extensions.Ingress) bool { } return expected == actual.Status.LoadBalancer.Ingress[0].IP } + +func TestGenerateExternalEndpointsFromStatus(t *testing.T) { + su := statusUpdater{ + status: []v1.LoadBalancerIngress{ + { + IP: "8.8.8.8", + }, + }, + } + + expectedEndpoints := []conf_v1.ExternalEndpoint{ + {IP: "8.8.8.8", Ports: ""}, + } + + endpoints := su.generateExternalEndpointsFromStatus(su.status) + + if !reflect.DeepEqual(endpoints, expectedEndpoints) { + t.Errorf("generateExternalEndpointsFromStatus(%v) returned %v but expected %v", su.status, endpoints, expectedEndpoints) + } +} + +func TestHasVsStatusChanged(t *testing.T) { + state := "Valid" + reason := "AddedOrUpdated" + msg := "Configuration was added or updated" + + tests := []struct { + expected bool + vs conf_v1.VirtualServer + }{ + { + expected: false, + vs: conf_v1.VirtualServer{ + Status: conf_v1.VirtualServerStatus{ + State: state, + Reason: reason, + Message: msg, + }, + }, + }, + { + expected: true, + vs: conf_v1.VirtualServer{ + Status: conf_v1.VirtualServerStatus{ + State: "DifferentState", + Reason: reason, + Message: msg, + }, + }, + }, + { + expected: true, + vs: conf_v1.VirtualServer{ + Status: conf_v1.VirtualServerStatus{ + State: state, + Reason: "DifferentReason", + Message: msg, + }, + }, + }, + { + expected: true, + vs: conf_v1.VirtualServer{ + Status: conf_v1.VirtualServerStatus{ + State: state, + Reason: reason, + Message: "DifferentMessage", + }, + }, + }, + } + + for _, test := range tests { + changed := hasVsStatusChanged(&test.vs, state, reason, msg) + + if changed != test.expected { + t.Errorf("hasVsStatusChanged(%v, %v, %v, %v) returned %v but expected %v.", test.vs, state, reason, msg, changed, test.expected) + } + } +} + +func TestHasVsrStatusChanged(t *testing.T) { + + referencedBy := "namespace/name" + state := "Valid" + reason := "AddedOrUpdated" + msg := "Configuration was added or updated" + + tests := []struct { + expected bool + vsr conf_v1.VirtualServerRoute + }{ + { + expected: false, + vsr: conf_v1.VirtualServerRoute{ + Status: conf_v1.VirtualServerRouteStatus{ + State: state, + Reason: reason, + Message: msg, + ReferencedBy: referencedBy, + }, + }, + }, + { + expected: true, + vsr: conf_v1.VirtualServerRoute{ + Status: conf_v1.VirtualServerRouteStatus{ + State: "DifferentState", + Reason: reason, + Message: msg, + ReferencedBy: referencedBy, + }, + }, + }, + { + expected: true, + vsr: conf_v1.VirtualServerRoute{ + Status: conf_v1.VirtualServerRouteStatus{ + State: state, + Reason: "DifferentReason", + Message: msg, + ReferencedBy: referencedBy, + }, + }, + }, + { + expected: true, + vsr: conf_v1.VirtualServerRoute{ + Status: conf_v1.VirtualServerRouteStatus{ + State: state, + Reason: reason, + Message: "DifferentMessage", + ReferencedBy: referencedBy, + }, + }, + }, + { + expected: true, + vsr: conf_v1.VirtualServerRoute{ + Status: conf_v1.VirtualServerRouteStatus{ + State: state, + Reason: reason, + Message: msg, + ReferencedBy: "DifferentReferencedBy", + }, + }, + }, + } + + for _, test := range tests { + changed := hasVsrStatusChanged(&test.vsr, state, reason, msg, referencedBy) + + if changed != test.expected { + t.Errorf("hasVsrStatusChanged(%v, %v, %v, %v) returned %v but expected %v.", test.vsr, state, reason, msg, changed, test.expected) + } + } +} + +func TestGetExternalServicePorts(t *testing.T) { + svc := v1.Service{ + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Port: int32(80), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + { + Port: int32(443), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 443, + }, + }, + }, + }, + } + + expected := "[80,443]" + ports := getExternalServicePorts(&svc) + + if ports != expected { + t.Errorf("getExternalServicePorts(%v) returned %v but expected %v", svc, ports, expected) + } +} + +func TestIsRequiredPort(t *testing.T) { + tests := []struct { + port intstr.IntOrString + expected bool + }{ + { + port: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 999, + }, + expected: false, + }, + { + port: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + expected: true, + }, + { + port: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 443, + }, + expected: true, + }, + { + port: intstr.IntOrString{ + Type: intstr.String, + StrVal: "name", + }, + expected: false, + }, + { + port: intstr.IntOrString{ + Type: intstr.String, + StrVal: "http", + }, + expected: true, + }, + { + port: intstr.IntOrString{ + Type: intstr.String, + StrVal: "https", + }, + expected: true, + }, + } + + for _, test := range tests { + result := isRequiredPort(test.port) + + if result != test.expected { + t.Errorf("isRequiredPort(%+v) returned %v but expected %v", test.port, result, test.expected) + } + } +} diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index d2e134d84f..9ee54e19fd 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -4,6 +4,15 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + // StateWarning is used when the resource has been validated and accepted but it might work in a degraded state. + StateWarning = "Warning" + // StateValid is used when the resource has been validated and accepted and is working as expected. + StateValid = "Valid" + // StateInvalid is used when the resource failed validation or NGINX failed to reload the corresponding config. + StateInvalid = "Invalid" +) + // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +kubebuilder:validation:Optional @@ -13,7 +22,8 @@ type VirtualServer struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - Spec VirtualServerSpec `json:"spec"` + Spec VirtualServerSpec `json:"spec"` + Status VirtualServerStatus `json:"status"` } // VirtualServerSpec is the spec of the VirtualServer resource. @@ -180,6 +190,20 @@ type TLSRedirect struct { BasedOn string `json:"basedOn"` } +// VirtualServerStatus defines the status for the VirtualServer resource. +type VirtualServerStatus struct { + State string `json:"state"` + Reason string `json:"reason"` + Message string `json:"message"` + ExternalEndpoints []ExternalEndpoint `json:"externalEndpoints,omitempty"` +} + +// ExternalEndpoint defines the IP and ports used to connect to this resource. +type ExternalEndpoint struct { + IP string `json:"ip"` + Ports string `json:"ports"` +} + // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // VirtualServerList is a list of the VirtualServer resources. @@ -197,7 +221,8 @@ type VirtualServerRoute struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - Spec VirtualServerRouteSpec `json:"spec"` + Spec VirtualServerRouteSpec `json:"spec"` + Status VirtualServerRouteStatus `json:"status"` } type VirtualServerRouteSpec struct { @@ -220,3 +245,12 @@ type UpstreamQueue struct { Size int `json:"size"` Timeout string `json:"timeout"` } + +// VirtualServerRouteStatus defines the status for the VirtualServerRoute resource. +type VirtualServerRouteStatus struct { + State string `json:"state"` + Reason string `json:"reason"` + Message string `json:"message"` + ReferencedBy string `json:"referencedBy"` + ExternalEndpoints []ExternalEndpoint `json:"externalEndpoints,omitempty"` +} diff --git a/pkg/apis/configuration/v1/zz_generated.deepcopy.go b/pkg/apis/configuration/v1/zz_generated.deepcopy.go index 449d133bd2..731508732b 100644 --- a/pkg/apis/configuration/v1/zz_generated.deepcopy.go +++ b/pkg/apis/configuration/v1/zz_generated.deepcopy.go @@ -152,6 +152,22 @@ func (in *ErrorPageReturn) DeepCopy() *ErrorPageReturn { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ExternalEndpoint) DeepCopyInto(out *ExternalEndpoint) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalEndpoint. +func (in *ExternalEndpoint) DeepCopy() *ExternalEndpoint { + if in == nil { + return nil + } + out := new(ExternalEndpoint) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Header) DeepCopyInto(out *Header) { *out = *in @@ -466,6 +482,7 @@ func (in *VirtualServer) DeepCopyInto(out *VirtualServer) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) return } @@ -526,6 +543,7 @@ func (in *VirtualServerRoute) DeepCopyInto(out *VirtualServerRoute) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) return } @@ -610,6 +628,27 @@ func (in *VirtualServerRouteSpec) DeepCopy() *VirtualServerRouteSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VirtualServerRouteStatus) DeepCopyInto(out *VirtualServerRouteStatus) { + *out = *in + if in.ExternalEndpoints != nil { + in, out := &in.ExternalEndpoints, &out.ExternalEndpoints + *out = make([]ExternalEndpoint, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VirtualServerRouteStatus. +func (in *VirtualServerRouteStatus) DeepCopy() *VirtualServerRouteStatus { + if in == nil { + return nil + } + out := new(VirtualServerRouteStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VirtualServerSpec) DeepCopyInto(out *VirtualServerSpec) { *out = *in @@ -644,3 +683,24 @@ func (in *VirtualServerSpec) DeepCopy() *VirtualServerSpec { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VirtualServerStatus) DeepCopyInto(out *VirtualServerStatus) { + *out = *in + if in.ExternalEndpoints != nil { + in, out := &in.ExternalEndpoints, &out.ExternalEndpoints + *out = make([]ExternalEndpoint, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VirtualServerStatus. +func (in *VirtualServerStatus) DeepCopy() *VirtualServerStatus { + if in == nil { + return nil + } + out := new(VirtualServerStatus) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/client/clientset/versioned/typed/configuration/v1/fake/fake_virtualserver.go b/pkg/client/clientset/versioned/typed/configuration/v1/fake/fake_virtualserver.go index 9e36bc192b..9e0bdbd8be 100644 --- a/pkg/client/clientset/versioned/typed/configuration/v1/fake/fake_virtualserver.go +++ b/pkg/client/clientset/versioned/typed/configuration/v1/fake/fake_virtualserver.go @@ -86,6 +86,18 @@ func (c *FakeVirtualServers) Update(ctx context.Context, virtualServer *configur return obj.(*configurationv1.VirtualServer), err } +// UpdateStatus was generated because the type contains a Status member. +// Add a +genclient:noStatus comment above the type to avoid generating UpdateStatus(). +func (c *FakeVirtualServers) UpdateStatus(ctx context.Context, virtualServer *configurationv1.VirtualServer, opts v1.UpdateOptions) (*configurationv1.VirtualServer, error) { + obj, err := c.Fake. + Invokes(testing.NewUpdateSubresourceAction(virtualserversResource, "status", c.ns, virtualServer), &configurationv1.VirtualServer{}) + + if obj == nil { + return nil, err + } + return obj.(*configurationv1.VirtualServer), err +} + // Delete takes name of the virtualServer and deletes it. Returns an error if one occurs. func (c *FakeVirtualServers) Delete(ctx context.Context, name string, opts v1.DeleteOptions) error { _, err := c.Fake. diff --git a/pkg/client/clientset/versioned/typed/configuration/v1/fake/fake_virtualserverroute.go b/pkg/client/clientset/versioned/typed/configuration/v1/fake/fake_virtualserverroute.go index 60b4656d19..d6cd85d7d9 100644 --- a/pkg/client/clientset/versioned/typed/configuration/v1/fake/fake_virtualserverroute.go +++ b/pkg/client/clientset/versioned/typed/configuration/v1/fake/fake_virtualserverroute.go @@ -86,6 +86,18 @@ func (c *FakeVirtualServerRoutes) Update(ctx context.Context, virtualServerRoute return obj.(*configurationv1.VirtualServerRoute), err } +// UpdateStatus was generated because the type contains a Status member. +// Add a +genclient:noStatus comment above the type to avoid generating UpdateStatus(). +func (c *FakeVirtualServerRoutes) UpdateStatus(ctx context.Context, virtualServerRoute *configurationv1.VirtualServerRoute, opts v1.UpdateOptions) (*configurationv1.VirtualServerRoute, error) { + obj, err := c.Fake. + Invokes(testing.NewUpdateSubresourceAction(virtualserverroutesResource, "status", c.ns, virtualServerRoute), &configurationv1.VirtualServerRoute{}) + + if obj == nil { + return nil, err + } + return obj.(*configurationv1.VirtualServerRoute), err +} + // Delete takes name of the virtualServerRoute and deletes it. Returns an error if one occurs. func (c *FakeVirtualServerRoutes) Delete(ctx context.Context, name string, opts v1.DeleteOptions) error { _, err := c.Fake. diff --git a/pkg/client/clientset/versioned/typed/configuration/v1/virtualserver.go b/pkg/client/clientset/versioned/typed/configuration/v1/virtualserver.go index 2dab4e56c4..4d9b39944a 100644 --- a/pkg/client/clientset/versioned/typed/configuration/v1/virtualserver.go +++ b/pkg/client/clientset/versioned/typed/configuration/v1/virtualserver.go @@ -24,6 +24,7 @@ type VirtualServersGetter interface { type VirtualServerInterface interface { Create(ctx context.Context, virtualServer *v1.VirtualServer, opts metav1.CreateOptions) (*v1.VirtualServer, error) Update(ctx context.Context, virtualServer *v1.VirtualServer, opts metav1.UpdateOptions) (*v1.VirtualServer, error) + UpdateStatus(ctx context.Context, virtualServer *v1.VirtualServer, opts metav1.UpdateOptions) (*v1.VirtualServer, error) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error DeleteCollection(ctx context.Context, opts metav1.DeleteOptions, listOpts metav1.ListOptions) error Get(ctx context.Context, name string, opts metav1.GetOptions) (*v1.VirtualServer, error) @@ -119,6 +120,22 @@ func (c *virtualServers) Update(ctx context.Context, virtualServer *v1.VirtualSe return } +// UpdateStatus was generated because the type contains a Status member. +// Add a +genclient:noStatus comment above the type to avoid generating UpdateStatus(). +func (c *virtualServers) UpdateStatus(ctx context.Context, virtualServer *v1.VirtualServer, opts metav1.UpdateOptions) (result *v1.VirtualServer, err error) { + result = &v1.VirtualServer{} + err = c.client.Put(). + Namespace(c.ns). + Resource("virtualservers"). + Name(virtualServer.Name). + SubResource("status"). + VersionedParams(&opts, scheme.ParameterCodec). + Body(virtualServer). + Do(ctx). + Into(result) + return +} + // Delete takes name of the virtualServer and deletes it. Returns an error if one occurs. func (c *virtualServers) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error { return c.client.Delete(). diff --git a/pkg/client/clientset/versioned/typed/configuration/v1/virtualserverroute.go b/pkg/client/clientset/versioned/typed/configuration/v1/virtualserverroute.go index db3929261f..76290fd5eb 100644 --- a/pkg/client/clientset/versioned/typed/configuration/v1/virtualserverroute.go +++ b/pkg/client/clientset/versioned/typed/configuration/v1/virtualserverroute.go @@ -24,6 +24,7 @@ type VirtualServerRoutesGetter interface { type VirtualServerRouteInterface interface { Create(ctx context.Context, virtualServerRoute *v1.VirtualServerRoute, opts metav1.CreateOptions) (*v1.VirtualServerRoute, error) Update(ctx context.Context, virtualServerRoute *v1.VirtualServerRoute, opts metav1.UpdateOptions) (*v1.VirtualServerRoute, error) + UpdateStatus(ctx context.Context, virtualServerRoute *v1.VirtualServerRoute, opts metav1.UpdateOptions) (*v1.VirtualServerRoute, error) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error DeleteCollection(ctx context.Context, opts metav1.DeleteOptions, listOpts metav1.ListOptions) error Get(ctx context.Context, name string, opts metav1.GetOptions) (*v1.VirtualServerRoute, error) @@ -119,6 +120,22 @@ func (c *virtualServerRoutes) Update(ctx context.Context, virtualServerRoute *v1 return } +// UpdateStatus was generated because the type contains a Status member. +// Add a +genclient:noStatus comment above the type to avoid generating UpdateStatus(). +func (c *virtualServerRoutes) UpdateStatus(ctx context.Context, virtualServerRoute *v1.VirtualServerRoute, opts metav1.UpdateOptions) (result *v1.VirtualServerRoute, err error) { + result = &v1.VirtualServerRoute{} + err = c.client.Put(). + Namespace(c.ns). + Resource("virtualserverroutes"). + Name(virtualServerRoute.Name). + SubResource("status"). + VersionedParams(&opts, scheme.ParameterCodec). + Body(virtualServerRoute). + Do(ctx). + Into(result) + return +} + // Delete takes name of the virtualServerRoute and deletes it. Returns an error if one occurs. func (c *virtualServerRoutes) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error { return c.client.Delete(). diff --git a/tests/suite/test_v_s_route_upstream_options.py b/tests/suite/test_v_s_route_upstream_options.py index 58ff983a48..91c9382cb0 100644 --- a/tests/suite/test_v_s_route_upstream_options.py +++ b/tests/suite/test_v_s_route_upstream_options.py @@ -155,7 +155,7 @@ def test_when_option_in_config_map_only(self, kube_apis, replace_configmap_from_yaml(kube_apis.v1, config_map_name, ingress_controller_prerequisites.namespace, config_map_file) - wait_before_test(1) + wait_before_test() ic_pod_name = get_first_pod_name(kube_apis.v1, ingress_controller_prerequisites.namespace) config = get_vs_nginx_template_conf(kube_apis.v1, v_s_route_setup.namespace,