<p dir="auto">Hey,</p>
<p dir="auto">I've noticed that on 1.3.0-beta.1 when petset is deleted its pods stay alive:</p>
<p dir="auto">core@master ~ $ kubectl get petsets | grep mariadb<br>
mariadb-galera   3         3         7s<br>
core@master ~ $ kubectl get pods | grep mariadb-galera<br>
mariadb-galera-0                 1/1       Running       0          5s<br>
mariadb-galera-1                 1/1       Running       0          4s<br>
mariadb-galera-2                 1/1       Running       0          3s<br>
core@master ~ $ kubectl delete petset mariadb-galera<br>
petset "mariadb-galera" deleted<br>
core@master ~ $ kubectl get pods | grep mariadb-galera<br>
mariadb-galera-0                 1/1       Running       0          18s<br>
mariadb-galera-1                 1/1       Running       0          17s<br>
mariadb-galera-2                 1/1       Running       0          16s<br>
core@master ~ $ kubectl get petsets | grep mariadb-galera<br>
core@master ~ $</p>
<p dir="auto">Cheers</p>
<p dir="auto">As part of <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="45429704" data-permission-text="Title is private" data-url="https://github.com/kubernetes/kubernetes/issues/1702" data-hovercard-type="issue" data-hovercard-url="/kubernetes/kubernetes/issues/1702/hovercard" href="https://github.com/kubernetes/kubernetes/issues/1702">#1702</a>, we would like to use declarative configuration to maintain the state of a Kubernetes cluster. We do this by issuing the local objects to the server as JSON HTTP PATCH requests. As per the <a href="https://tools.ietf.org/html/rfc7386" rel="nofollow">JSON merge patch spec</a>, all maps are merged together, keys that are omitted from the patch are left untouched in the document, and if you want to delete a key you simply use <code class="notranslate">key: null</code>. This is exactly what we want in config reconciliation. However, lists present a special challenge in that the spec says they should always be replaced. Often that isn't what we want.</p>
<p dir="auto">Let's say we start with the following Pod:</p>
<div class="highlight highlight-source-yaml notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content="spec:
  containers:
    - name: nginx
      image: nginx-1.0"><pre class="notranslate"><span class="pl-ent">spec</span>:
  <span class="pl-ent">containers</span>:
    - <span class="pl-ent">name</span>: <span class="pl-s">nginx</span>
      <span class="pl-ent">image</span>: <span class="pl-s">nginx-1.0</span></pre></div>
<p dir="auto">...and we POST that to the server. Then a controller comes along and adds another container to this pod before it starts up. (Not possible today but likely a scenario we will want to support.)</p>
<div class="highlight highlight-source-yaml notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content="spec:
  containers:
    - name: nginx
      image: nginx-1.0
    - name: log-tailer
      image: log-tailer-1.0"><pre class="notranslate"><span class="pl-ent">spec</span>:
  <span class="pl-ent">containers</span>:
    - <span class="pl-ent">name</span>: <span class="pl-s">nginx</span>
      <span class="pl-ent">image</span>: <span class="pl-s">nginx-1.0</span>
    - <span class="pl-ent">name</span>: <span class="pl-s">log-tailer</span>
      <span class="pl-ent">image</span>: <span class="pl-s">log-tailer-1.0</span></pre></div>
<p dir="auto">If you try to apply the local config as a JSON PATCH to the remote config, you <em>should</em> get a no-op: the container that we've specified (nginx) still exists in the server, and extra containers inside the <code class="notranslate">containers</code> field are not of concern to us (and we should not clobber them). However, there is no way to programatically see if this is a list that should be merged versus a list that should <em>not</em> be merged. Here is an example of a list that should <em>not</em> be merged (the <code class="notranslate">ExecAction</code> API object), and instead should be fully replaced when it is modified:</p>
<div class="highlight highlight-source-yaml notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content="command:
  - nginx
  - -t
  - -c
  - /etc/nginx.conf"><pre class="notranslate"><span class="pl-ent">command</span>:
  - <span class="pl-s">nginx</span>
  - <span class="pl-s">-t</span>
  - <span class="pl-s">-c</span>
  - <span class="pl-s">/etc/nginx.conf</span></pre></div>
<p dir="auto">If the remote server were to look like this...</p>
<div class="highlight highlight-source-yaml notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content="command:
  - httpd
  - -X
  - -f
  - /etc/httpd.conf"><pre class="notranslate"><span class="pl-ent">command</span>:
  - <span class="pl-s">httpd</span>
  - <span class="pl-s">-X</span>
  - <span class="pl-s">-f</span>
  - <span class="pl-s">/etc/httpd.conf</span></pre></div>
<p dir="auto">...then if I were to patch the remote server with the local config, it should NOT merge these lists together - it should actually replace the <code class="notranslate">command</code> field wholesale with the one on disk.</p>
<p dir="auto">The semantic knowledge of which lists should be merged like maps versus which lists should be replaced completely belongs in the API server. There are two ways to accomplish this.</p>
<h3 dir="auto">Change mergeable lists to maps</h3>
<p dir="auto">If a list is mergeable, we should make it a map in the API server of a key to the objects. See a thorough discussion of this alternative to lists in <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="46937389" data-permission-text="Title is private" data-url="https://github.com/kubernetes/kubernetes/issues/2004" data-hovercard-type="issue" data-hovercard-url="/kubernetes/kubernetes/issues/2004/hovercard?comment_id=60641437&amp;comment_type=issue_comment" href="https://github.com/kubernetes/kubernetes/issues/2004#issuecomment-60641437">#2004 (comment)</a>. The above example with pod spec would transform to this:</p>
<div class="highlight highlight-source-yaml notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content="spec:
  containers:
    nginx:
      image: nginx-1.0"><pre class="notranslate"><span class="pl-ent">spec</span>:
  <span class="pl-ent">containers</span>:
    <span class="pl-ent">nginx</span>:
      <span class="pl-ent">image</span>: <span class="pl-s">nginx-1.0</span></pre></div>
<p dir="auto">And the remote side would look like this:</p>
<div class="highlight highlight-source-yaml notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content="spec:
  containers:
    nginx:
      image: nginx-1.0
    log-tailer:
      image: log-tailer-1.0"><pre class="notranslate"><span class="pl-ent">spec</span>:
  <span class="pl-ent">containers</span>:
    <span class="pl-ent">nginx</span>:
      <span class="pl-ent">image</span>: <span class="pl-s">nginx-1.0</span>
    <span class="pl-ent">log-tailer</span>:
      <span class="pl-ent">image</span>: <span class="pl-s">log-tailer-1.0</span></pre></div>
<p dir="auto">This enables us to merge the maps properly. If you wanted to actually delete the <code class="notranslate">nginx</code> container, you would put <code class="notranslate">nginx: null</code> in the local patch. (We have a design which will automatically generate the correct nulls of things you've deleted based on an annotation containing your previously submitted config - this design will be written into <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="45429704" data-permission-text="Title is private" data-url="https://github.com/kubernetes/kubernetes/issues/1702" data-hovercard-type="issue" data-hovercard-url="/kubernetes/kubernetes/issues/1702/hovercard" href="https://github.com/kubernetes/kubernetes/issues/1702">#1702</a>.)</p>
<p dir="auto">The big advantage of this method though is that it's really obvious what gets merged and what doesn't (lists are always replaced, maps are always merged) and we can use a standard JSON PATCH library to accept patches on the server and we can directly conform to the <a href="https://tools.ietf.org/html/rfc7386" rel="nofollow">JSON merge patch spec</a>. A side benefit is that the list is readily accessible in a meaningful way (indexed by primary key) which avoids manually indexing lists all the time.</p>
<p dir="auto">The downside (as outlined in <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/jbeda/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/jbeda">@jbeda</a>'s comment) is that the field to use as the key for the map is not always obvious. We can implement it as a swagger annotation to mitigate that. The bigger issue is that if a struct has no other required fields besides the map field, it can get awkward. For example, the PublicIP's field of the service spec is likely something we would want to merge. It's a <code class="notranslate">[]string</code> which looks like this:</p>
<div class="highlight highlight-source-yaml notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content="spec:
 publicIPs:
    - 67.1.1.10
    - 67.1.1.11"><pre class="notranslate"><span class="pl-ent">spec</span>:
 <span class="pl-ent">publicIPs</span>:
    - <span class="pl-s">67.1.1.10</span>
    - <span class="pl-s">67.1.1.11</span></pre></div>
<p dir="auto">If we made it a mergeable map (since you may want someone to inject a public IP into a service), then we'd have to change it to look like something like this:</p>
<div class="highlight highlight-source-yaml notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content="spec:
 publicIPs:
    67.1.1.10:
      ip: 67.1.1.10
    67.1.1.11:
      ip: 67.1.1.11"><pre class="notranslate"><span class="pl-ent">spec</span>:
 <span class="pl-ent">publicIPs</span>:
    <span class="pl-ent">67.1.1.10</span>:
      <span class="pl-ent">ip</span>: <span class="pl-s">67.1.1.10</span>
    <span class="pl-ent">67.1.1.11</span>:
      <span class="pl-ent">ip</span>: <span class="pl-s">67.1.1.11</span></pre></div>
<p dir="auto">There is an optimization we could do where IP would be automatically filled by API server as a default, which would make it look like this:</p>
<div class="highlight highlight-source-yaml notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content="spec:
 publicIPs:
    67.1.1.10: {}
    67.1.1.11: {}"><pre class="notranslate"><span class="pl-ent">spec</span>:
 <span class="pl-ent">publicIPs</span>:
    <span class="pl-ent">67.1.1.10</span>: <span class="pl-s">{}</span>
    <span class="pl-ent">67.1.1.11</span>: <span class="pl-s">{}</span></pre></div>
<p dir="auto">But in any case it's still a bit ugly. There may be other ways to beautify it and/or solve this problem - very open to suggestions here.</p>
<h3 dir="auto">Swagger annotation to tell API server which lists should be merged</h3>
<p dir="auto">The alternative is to do the merge determination more dynamically. When API server gets a PATCH, it introspects into the lists and checks against a Swagger annotation to see where the list should be replaced or merged. If it's one that should be merged, it will need a mergeKey which tells it what key to use to do the merging. In the above case of containers, it would be <code class="notranslate">name</code>, but in the publicIP's case it would just uniqify and merge the values themselves. While this lets us not change the API, this requires us to implement and maintain a custom JSON PATCH implementation which will not be fun. It also goes against POLA when your PATCH request will not behave as <a href="https://tools.ietf.org/html/rfc7386" rel="nofollow">the spec</a> defines.</p>
<h3 dir="auto">What should we do?</h3>
<p dir="auto">Neither of these options are fantastic, but I think we need to implement this distinction as part of the API. Please discuss and detail other options if you can think of any.</p>
<p dir="auto"><a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/bgrant0607/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/bgrant0607">@bgrant0607</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/smarterclayton/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/smarterclayton">@smarterclayton</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/derekwaynecarr/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/derekwaynecarr">@derekwaynecarr</a> <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/mikedanese/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/mikedanese">@mikedanese</a></p>