cp.cpToPod() does not work

[w3ichen]

Describe the bug
cp.cpToPod() does not actually copy the file to the pod. No error is raised and the function resolves, that is, there is no indication that the copying had failed. However, the file does not exist in the pod.

After inspecting the source code, I found that the function resolves because a Promise is not used.
I found that the root cause is because the const command = ['tar', 'xf', '-', '-C', tgtPath]; hangs forever and displays no errors. It seems to me that the command tar xf -C is waiting for something and hangs infinitely.

Client Version
"@kubernetes/client-node": "^0.18.0"

To Reproduce

1. Create a pod called in nginx-4217019353-9gl4s in default namespace with container called nginx.
2. Use example cp code

import * as k8s from '@kubernetes/client-node';

const kc = new k8s.KubeConfig();
kc.loadFromDefault();

const cp = new k8s.Cp(kc);
cp.cpFromPod('default', 'nginx-4217019353-9gl4s', 'nginx', './test.txt', '/tmp');

3. Connect to the pod and inspect for the file

kubectl exec --stdin --tty nginx-4217019353-9gl4s -- /bin/bash
cd /tmp
ls test.txt

Expected behavior
I expect the file to be copied into the pod, and for the file to exist in the pod.

Environment:

• OS: MacOS
• NodeJS Version: v16.17.0
• Cloud runtime: DigitalOcean Kubernetes

Additional context
I'm currently using a hack to work around this by spawning a process to call kubectl cp.
However, it's not ideal since it depends on the kubectl command line tool:

import * as k8s from "@kubernetes/client-node";
import { Cp } from "@kubernetes/client-node";
import { spawn } from "child_process";
import * as fs from "fs";
import * as tmp from "tmp-promise";

export class CustomCp extends Cp {
   /**
   * Modified version of cpToPod() to copy files from a string to pod without writing files
   * NOTE: the cpToPod() does not seem to work (no copy made). It resolves but the tar command hangs.
   * @param {string} namespace - The namespace of the pod to exec the command inside.
   * @param {string} podName - The name of the pod to exec the command inside.
   * @param {string} containerName - The name of the container in the pod to exec the command inside.
   * @param {string} srcString - The source string to copy
   * @param {string} tgtPath - The target path in the pod
   */
  public async cpStringToPod(
    namespace: string,
    podName: string,
    containerName: string,
    srcString: string,
    tgtPath: string
  ): Promise<void> {
    return new Promise(async function (resolve, reject) {
      // (1) Create a temporary file
      const tmpFile = tmp.fileSync();
      const tmpFilename = tmpFile.name;
      // (2) Write the string to the temporary file
      await fs.writeFileSync(tmpFilename, srcString);
      // (3) Copy the temporary file to the pod using `kubectl cp` command line
      const cp = await spawn("kubectl", ["cp", tmpFilename, `${namespace}/${podName}:${tgtPath}`, "-c", containerName]);
      cp.stderr.on("data", (data) => {
        tmpFile.removeCallback(); // remove file
        reject(data);
      });

      cp.on("close", (code) => {
        tmpFile.removeCallback(); // remove file
        if (code == 0) resolve();
        else reject(`exited with code ${code}`);
      });
    });
  }
}
export const cp = new CustomCp(kc, exec);
cp.cpStringToPod(...)

[SayakMukhopadhyay]

I am facing the same issue. In my case, the js code running this library is my local system and the target pod is on minikube. I have not tested this when both the js code running this library is also on k8s. Anyway, an observation I have made is that cpToPod has a last line as

javascript/src/cp.ts

Line 81 in 6aa7539

this.execInstance.exec(

The last line doesn't call exec with an await. Since the function returns void, the calling function of cpToPod has no way to wait for the copy to be completed as this.execInstance.exec() will return immediately.

When I test locally by adding an await (to make it await this.execInstance.exec(...), then cpToPod just hangs as noted by the OP.

[w3ichen]

@vip30 kindly suggest any changes?

[SayakMukhopadhyay]

@w3ichen I think I have figured out a reason why this could happen. After much trial and error, I noticed that the copy was actually working if I wait for the pod to get into the Running phase before I call the cp.cpToPod function. I have made a PoC that shows how this works at https://github.com/SayakMukhopadhyay/k8s-cp-test.

Maybe the maintainers would be able to confirm if my observations are correct.

EDIT: Upon further debugging the above project, I see that the items are not copied until the program exits. I have put a breakpoint in console.log('Done') and have waited for varying amounts of time but don't see the copied items. I see them as soon as the program exits. I am not sure what is happening here. Some insight would be great!

[SayakMukhopadhyay]

I think I have found out why the program wants to exit before the copying is completed. I created the above PR to fix that. @w3ichen maybe you can check that PR and see if that works for you.

[w3ichen]

@SayakMukhopadhyay I tested your PR and it works for me. The way you're doing it by listening to when the websocket closes seems like the only way right now because the exec will resolve once the websocket is established and it doesn't call any callback functions.

[w3ichen]

I'm closing this issue now because the cpToPod does work but only in certain conditionals that were not clear at first.

In order for cpToPod to work:

1. The pod must be "running".
2. tgtPath must be a directory and not a filename. This was causing it to silently fail on me before.
3. The statusCallback is never called (ie. async ({ status }) => { ... }). This led me to think that it was hanging but it does resolve eventually.

Improvements:

1. cpToPod will resolve without waiting for the copying to complete. This can be fixed by using a Promise as @SayakMukhopadhyay suggested.
2. The current function makes use of temporary files and requires actual files to exist. For me, I wanted to avoid reading/writing to files for speedup and I needed to use strings. So I extended the Cp class with cpFromPodToString and cpStringToPod as follows:

import * as k8s from "@kubernetes/client-node";
import { Cp } from "@kubernetes/client-node";
import { WritableStreamBuffer } from "stream-buffers";
import { Callback, Headers, pack } from "tar-stream";

export type TarEntry = [Headers, string, Callback?];

export class CustomCp extends Cp {
  public async cpFromPodToString(namespace: string, podName: string, containerName: string, srcPath: string): Promise<string> {
    return new Promise((resolve, reject) => {
      const command = ["cat", srcPath];
      const writerStream = new WritableStreamBuffer();
      const errStream = new WritableStreamBuffer();
      this.execInstance.exec(
        namespace,
        podName,
        containerName,
        command,
        writerStream,
        errStream,
        null,
        false,
        async ({ status }) => {
          if (status === "Failure" || errStream.size())
            reject(`Error from cpFromPodToString - details: \n ${errStream.getContentsAsString()}`);
          resolve(writerStream.getContentsAsString() || "");
        }
      );
    });
  }

  public async cpStringToPod(
    namespace: string,
    podName: string,
    containerName: string,
    srcFiles: TarEntry[],
    tgtPath: string
  ): Promise<void> {
    const readStream = pack();
    srcFiles.forEach((tarEntry) => {
      readStream.entry(...tarEntry);
    });
    readStream.finalize();

    const command = ["tar", "xf", "-", "-C", tgtPath];

    const errStream = new WritableStreamBuffer();
    const conn = await this.execInstance.exec(
      namespace,
      podName,
      containerName,
      command,
      null,
      errStream,
      readStream,
      false,
      async ({ status }) => {
        // Does not reach here for unknown reasons
        if (status === "Failure" || errStream.size()) {
          throw new Error(`Error from cpStringToPod - details: \n ${errStream.getContentsAsString()}`);
        }
      }
    );

    return new Promise((resolve) => {
      conn.onclose = (event) => {
        resolve();
      };
    });
  }
}

const kc = new k8s.KubeConfig();
kc.loadFromDefault();
const exec = new k8s.Exec(kc);

export const cp = new CustomCp(kc, exec);

[SayakMukhopadhyay]

@w3ichen Could you please keep the issue opened since I have the same issue and I want to ensure that the maintainers see this as an open item and not something that has been "fixed".

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[SayakMukhopadhyay]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten