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

Implement provisioner #683

Merged
merged 1 commit into from
Jun 1, 2023
Merged

Conversation

pleshakov
Copy link
Contributor

@pleshakov pleshakov commented May 25, 2023

Proposed changes

Problem:
Support provisioning NGINX data plane per Gateway, as expected by the Gateway API conformance tests.
See #634 for more info

Solution:

  • Implement provisioner command which which provisions a Deployment of NKG (static mode) for each Gateway of the provisioner GatewayClass.
  • Add provisioner manifests and docs

Fixes #634

Additionally, introduce PrepareTimeForFakeClient helper function, which
fixes an error that appeared on GitHub Action pipeline, not locally (see below).
(To reproduce it locally, run make TZ=123 unit-tests from an earlier commit
and ensure you
compare Conditions in the status as in
Expect(clusterGc.Status.Conditions).To(Equal(expectedConditions)) )

The timezone of the time in a resource field returned by the fake client was different
from the one set in the field when updating the resource.

The commit adds PrepareTimeForFakeClient() which ensures that the time is prepared
correctly so that the timezone is the same.

The problem is only present when comparing status Conditions using gomega like

Expect(clusterGc.Status.Conditions).To(Equal(expectedConditions))

but not present if comparing using cmp like

Expect(helpers.Diff(expectedGc, latestGc)).To(BeEmpty()).
  [FAILED] Expected
      <*time.Location | 0x30d0b00>: {
          name: "Local",
          zone: [
              {name: "UTC", offset: 0, isDST: false},
          ],
          tx: [
              {
                  when: -9223372036854775808,
                  index: 0,
                  isstd: false,
                  isutc: false,
              },
          ],
          extend: "UTC0",
          cacheStart: -9223372036854775808,
          cacheEnd: 9223372036854775807,
          cacheZone: {name: "UTC", offset: 0, isDST: false},
      }
  to equal
      <*time.Location | 0x309f240>: {name: "UTC", zone: nil, tx: nil, extend: "", cacheStart: 0, cacheEnd: 0, cacheZone: nil}

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@pleshakov pleshakov requested a review from a team as a code owner May 25, 2023 21:27
@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request labels May 25, 2023
@pleshakov pleshakov force-pushed the feature/provisioner-mode branch 2 times, most recently from 5522174 to dc88dc6 Compare June 1, 2023 01:38
@pleshakov pleshakov requested review from sjberman and kate-osborn June 1, 2023 01:43
@pleshakov pleshakov force-pushed the feature/provisioner-mode branch from dc88dc6 to 1463257 Compare June 1, 2023 21:47
Problem:
Support provisioning NGINX data plane per Gateway, as expected
by the Gateway API conformance tests.
See nginx#634
for more info

Solution:
- Implement provisioner command which which provisions a Deployment
of NKG (static mode) for each Gateway of the provisioner GatewayClass.
- Add provisioner manifests and docs

Fixes nginx#634

Additionally, introduce PrepareTimeForFakeClient helper function, which
fixes an error that appeared on GitHub Action pipeline, not locally (see below).
(To reproduce it locally, run `make TZ=123 unit-tests` and ensure you
compare Conditions in the status as in
Expect(clusterGc.Status.Conditions).To(Equal(expectedConditions)) )

The timezone of the time in a resource field returned by the fake client was different
from the one set in the field when updating the resource.

The commit adds PrepareTimeForFakeClient() which ensures that the time is prepared
correctly so that the timezone is the same.

The problem is only present when comparing status Conditions  using gomega like
Expect(clusterGc.Status.Conditions).To(Equal(expectedConditions))
but not present if comparing using cmp like
Expect(helpers.Diff(expectedGc, latestGc)).To(BeEmpty()).

  [FAILED] Expected
      <*time.Location | 0x30d0b00>: {
          name: "Local",
          zone: [
              {name: "UTC", offset: 0, isDST: false},
          ],
          tx: [
              {
                  when: -9223372036854775808,
                  index: 0,
                  isstd: false,
                  isutc: false,
              },
          ],
          extend: "UTC0",
          cacheStart: -9223372036854775808,
          cacheEnd: 9223372036854775807,
          cacheZone: {name: "UTC", offset: 0, isDST: false},
      }
  to equal
      <*time.Location | 0x309f240>: {name: "UTC", zone: nil, tx: nil, extend: "", cacheStart: 0, cacheEnd: 0, cacheZone: nil}
@pleshakov pleshakov force-pushed the feature/provisioner-mode branch from 1463257 to c610288 Compare June 1, 2023 22:02
@pleshakov pleshakov self-assigned this Jun 1, 2023
@pleshakov pleshakov merged commit 1df6bcd into nginx:main Jun 1, 2023
@pleshakov pleshakov deleted the feature/provisioner-mode branch June 1, 2023 22:07
miledxz added a commit to miledxz/nginx-gateway-fabric that referenced this pull request Jan 14, 2025
Problem:
Support provisioning NGINX data plane per Gateway, as expected
by the Gateway API conformance tests.
See nginx#634
for more info

Solution:
- Implement provisioner command which which provisions a Deployment
of NKG (static mode) for each Gateway of the provisioner GatewayClass.
- Add provisioner manifests and docs

Fixes nginx#634

Additionally, introduce PrepareTimeForFakeClient helper function, which
fixes an error that appeared on GitHub Action pipeline, not locally (see below).
(To reproduce it locally, run `make TZ=123 unit-tests` from a commit before
this one and ensure you compare Conditions in the status as in
Expect(clusterGc.Status.Conditions).To(Equal(expectedConditions)) )

The timezone of the time in a resource field returned by the fake client was different
from the one set in the field when updating the resource.

The commit adds PrepareTimeForFakeClient() which ensures that the time is prepared
correctly so that the timezone is the same.

The problem is only present when comparing status Conditions  using gomega like
Expect(clusterGc.Status.Conditions).To(Equal(expectedConditions))
but not present if comparing using cmp like
Expect(helpers.Diff(expectedGc, latestGc)).To(BeEmpty()).

  [FAILED] Expected
      <*time.Location | 0x30d0b00>: {
          name: "Local",
          zone: [
              {name: "UTC", offset: 0, isDST: false},
          ],
          tx: [
              {
                  when: -9223372036854775808,
                  index: 0,
                  isstd: false,
                  isutc: false,
              },
          ],
          extend: "UTC0",
          cacheStart: -9223372036854775808,
          cacheEnd: 9223372036854775807,
          cacheZone: {name: "UTC", offset: 0, isDST: false},
      }
  to equal
      <*time.Location | 0x309f240>: {name: "UTC", zone: nil, tx: nil, extend: "", cacheStart: 0, cacheEnd: 0, cacheZone: nil}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add data plane provisioner to support conformance tests
3 participants