Commit Graph

8 Commits

Author SHA1 Message Date
Tim Hockin
237434bd42 Svc REST: Overhaul Create test wrt dual-stack
This includes a few cases.

1) TestCreateIgnoresIPFamilyForExternalName: Prove that ExternalName is
ignored for dual-stack.  A small set of test cases were chosen to
demonstrate.

2) TestCreateIgnoresIPFamilyWithoutDualStack: Prove that when the
dual-stack gate is off, all services are ignored for dual-stack.  A
small set of test cases were chosen to demonstrate

3) TestCreateInitIPFields: Run over a huge array of test cases for
dual-stack.  This was generated by this program:
https://gist.github.com/thockin/cccc9c9a580b4830ee0946ddd43eeafe and
then updated by hand.
2021-09-11 10:56:03 -07:00
Tim Hockin
73503a4936 Fix a small regression in Service updates
Prior to 1.22 a user could change NodePort values within a service
during an update, and the apiserver would allocate values for any that
were not specified.

Consider a YAML like:

```
apiVersion: v1
kind: Service
metadata:
  name: foo
spec:
  type: NodePort
  ports:
  - name: p
    port: 80
  - name: q
    port: 81
  selector:
    app: foo
```

When this is created, nodeport values will be allocated for each port.
Something like:

```
apiVersion: v1
kind: Service
metadata:
  name: foo
spec:
  clusterIP: 10.0.149.11
  type: NodePort
  ports:
  - name: p
    nodePort: 30872
    port: 80
    protocol: TCP
    targetPort: 9376
  - name: q
    nodePort: 31310
    port: 81
    protocol: TCP
    targetPort: 81
  selector:
    app: foo
```

If the user PUTs (kubectl replace) the original YAML, we would see that
`.nodePort = 0`, and allocate new ports.  This was ugly at best.

In 1.22 we fixed this to not allocate new values if we still had the old
values, but instead re-assign them.  Net new ports would still be seen
as `.nodePort = 0` and so new allocations would be made.

This broke a corner case as follows:

Prior to 1.22, the user could PUT this YAML:

```
apiVersion: v1
kind: Service
metadata:
  name: foo
spec:
  type: NodePort
  ports:
  - name: p
    nodePort: 31310 # note this is the `q` value
    port: 80
  - name: q
    # note this nodePort is not specified
    port: 81
  selector:
    app: foo
```

The `p` port would take the `q` port's value.  The `q` port would be
seen as `.nodePort = 0` and a new value allocated.  In 1.22 this results
in an error (duplicate value in `p` and `q`).

This is VERY minor but it is an API regression, which we try to avoid,
and the fix is not too horrible.

This commit adds more robust testing of this logic.
2021-08-30 12:42:17 -07:00
Tim Hockin
80dda49ce2 Service: Fix semantics for Update wrt allocations
It is not uncommon for users to Create a Service and not specify things
like ClusterIP and NodePort, which we then allocate for them.  They same
that YAML somewhere and later use it again in an Update, but then it
fails.

That's because we detected them trying to set a ClusterIP from a value
to "", which is not allowed.  If it was just NodePort, they would
actually succeed and reallocate a new port.

After this change, we try to "patch" updates where the user did not
specify those values from the old object.
2021-07-07 17:09:12 -07:00
Tim Hockin
5b787aa184 Clean up testing of AllocateLoadBalancerNodePorts
We only need one "tweak" function, and it should be set automatically in
most cases.
2021-07-06 16:36:51 -07:00
Andrew Sy Kim
28f3f36505
Promote the ServiceInternalTrafficPolicy field to Beta and on by default (#103462)
* pkg/features: promote the ServiceInternalTrafficPolicy field to Beta and on by default

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>

* pkg/api/service/testing: update Service test fixture functions to set internalTrafficPolicy=Cluster by default

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>

* pkg/apis/core/validation: add more Service validation tests for internalTrafficPolicy

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>

* pkg/registry/core/service/storage: fix failing Service REST storage tests to use internalTrafficPolicy: Cluster

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>

* pkg/registry/core/service/storage: add two test cases for Service REST TestServiceRegistryInternalTrafficPolicyClusterThenLocal and TestServiceRegistryInternalTrafficPolicyLocalThenCluster

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>

* pkg/registry/core/service: update strategy unit tests to expect default
internalTrafficPolicy=Cluster

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>

* pkg/proxy/ipvs: fix unit test Test_EndpointSliceReadyAndTerminatingLocal to use internalTrafficPolicy=Cluster

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>

* pkg/apis/core: update fuzzers to set Service internalTrafficPolicy field

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>

* pkg/api/service/testing: refactor Service test fixtures to use Tweak funcs

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>
2021-07-06 06:16:30 -07:00
Hanlin Shi
24592ca989 Update the related tests
1. add AllocateLoadBalancerNodePorts fields in specs for validation test cases
2. update fuzzer
3. in resource quota e2e, allocate node port for loadbalancer type service and
   exceed the node port quota

Signed-off-by: Hanlin Shi <shihanlin9@gmail.com>
2021-07-02 21:58:41 +00:00
Tim Hockin
54b6a416fb Service REST test: better IP and port alloc checks 2021-07-01 23:01:36 -07:00
Tim Hockin
175f4f3387 Move service test-helper funcs to a package 2021-07-01 18:26:45 -07:00