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

Fix a small regression in Service updates #104601

Merged

Commits on Aug 30, 2021

  1. 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.
    thockin committed Aug 30, 2021
    Configuration menu
    Copy the full SHA
    73503a4 View commit details
    Browse the repository at this point in the history