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

cp.cpToPod() does not work #982

Closed
w3ichen opened this issue Feb 8, 2023 · 11 comments · May be fixed by #2038
Closed

cp.cpToPod() does not work #982

w3ichen opened this issue Feb 8, 2023 · 11 comments · May be fixed by #2038
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@w3ichen
Copy link

w3ichen commented Feb 8, 2023

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');
  1. 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
Copy link

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

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
Copy link
Author

w3ichen commented Feb 13, 2023

@vip30 kindly suggest any changes?

@SayakMukhopadhyay
Copy link

SayakMukhopadhyay commented Feb 14, 2023

@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
Copy link

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
Copy link
Author

w3ichen commented Feb 16, 2023

@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
Copy link
Author

w3ichen commented Feb 16, 2023

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);

@w3ichen w3ichen closed this as completed Feb 16, 2023
@SayakMukhopadhyay
Copy link

@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".

@w3ichen w3ichen reopened this Feb 16, 2023
@k8s-triage-robot
Copy link

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-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 17, 2023
@SayakMukhopadhyay
Copy link

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 20, 2023
@k8s-triage-robot
Copy link

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-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 20, 2024
@k8s-triage-robot
Copy link

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

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 19, 2024
@w3ichen w3ichen closed this as completed Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
4 participants