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

File system mapping doesn't apply #2894

Open
milad2golnia opened this issue Nov 5, 2024 · 16 comments
Open

File system mapping doesn't apply #2894

milad2golnia opened this issue Nov 5, 2024 · 16 comments

Comments

@milad2golnia
Copy link

milad2golnia commented Nov 5, 2024

Bug Description

I struggled with different mappings by feature.fs.mapping to see it in action but it don't take effect.

According to documentation:

"mapping" - Map of patterns and their corresponding replacers. The replacement happens before any specific behavior as defined above or mode (uses Regex::replace)

And in other hand:

Check agains “mapping” if path needs to be replaced, if matched then continue to next step with new path after replacements otherwise continue as usual.

An example is provided by documentation too:

{ "^/home/(?<user>\\S+)/dev/tomcat": "/etc/tomcat" "^/home/(?<user>\\S+)/dev/config/(?<app>\\S+)": "/mnt/configs/${user}-$app" }
Will do the next replacements for any io operaton
/home/johndoe/dev/tomcat/context.xml => /etc/tomcat/context.xml

Steps to Reproduce

  1. Here is my code:

  console.log('I am here')
  const child = spawn('cat', ['/home/johndoe/dev/tomcat/context.xml'])
  child.on('error', (err) => {
    console.log(err)
  })
  child.stderr.on('data', (chunk) => {
    console.log('stderr:', chunk)
  })
  child.stdout.on('data', (chunk) => {
    console.warn('stdout:', chunk)
  })
  1. And here is output which indicates there is no replacement:
stderr:
cat: /home/johndoe/dev/tomcat/context.xml: No such file or directory

Backtrace

No response

mirrord layer logs

No response

mirrord intproxy logs

No response

mirrord agent logs

No response

mirrord config

{
  "target": {
    "path": {
      "deployment": "my-pod"
    },
    "namespace": "services"
  },
  "feature": {
    "env": {
      "override": {
        "CATEGORY_ID": "1",
      }
    },
    "fs": {
      "mode": "write",
      "mapping": {
        "^/home/(?<user>\\S+)/dev/tomcat": "/etc/tomcat",
        "^/home/(?<user>\\S+)/dev/config/(?<app>\\S+)": "/mnt/configs/${user}-$app"
      }
    },
    "network": true
  },
  "agent": {
    "namespace": "services",
    "startup_timeout": 360
  },
  "kubeconfig": "~/.kube/test",
  "kube_context": "test"
}

mirrord CLI version

No response

mirrord-agent version

No response

mirrord-operator version (if relevant)

No response

plugin kind and version (if relevant)

No response

Your operating system and version

6.1.111-1-MANJARO

Local process

/usr/bin/cat: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=f279ce1a7add04b3f8b3f16e4fb89ecbc5d88dfb, for GNU/Linux 4.4.0, stripped

Local process version

ls (GNU coreutils) 9.5

Additional Info

Please let me know if any other information is required.
No response

@milad2golnia milad2golnia added the bug Something isn't working label Nov 5, 2024
Copy link

linear bot commented Nov 5, 2024

@aviramha
Copy link
Member

aviramha commented Nov 5, 2024

Do you have /etc/tomcat/context.xml in the remote pod?

@milad2golnia
Copy link
Author

milad2golnia commented Nov 6, 2024

I didn’t have it at first, but after your response, I created it, which was verified with the following command:

$ kubectl exec -n services my-pod-6f689d4c-zbxgc -it -- ls /etc/tomcat/context.xml -lah
-rw-r--r-- 1 root root 0 Nov  6 07:05 /etc/tomcat/context.xml

However, I’m still getting the same error when running mirrord inside VSCode:

stderr:
cat: /home/johndoe/dev/tomcat/context.xml: No such file or directory 

The mapping functionality of mirrord is really useful for testing projects. For example, it helps me test projects that contain relative paths.
I’m just trying to see the fs.mapping feature in action, and I’ve tried other mappings as well, but they don't work. If you'd prefer to test its functionality with a different example, please feel free to do so.

Also, I have another question:
Based on your response, mirrord may fail to map paths if the mapped file doesn’t exist in the pod. Am I correct?

@aviramha
Copy link
Member

aviramha commented Nov 6, 2024

I think you understood the mapping feature wrong - mapping is used to replace paths - i.e if you'd like /app/fun to become /wat/fun regardless of local/remote.
To mark a path as remote/local, you need to use the local/read_only settings inside fs

@milad2golnia
Copy link
Author

milad2golnia commented Nov 6, 2024

mapping is used to replace paths - i.e if you'd like /app/fun to become /wat/fun regardless of local/remote.

Sorry, You may got me wrong. I also expected this completely but seeing your response about existing file in remote, made me confused.

So mapping should occur regardless of existing file. But as you can see in output, the error message still contains old path and hence we can conclude mapping is not working correctly.

Actually I expect to see following output at least:

stderr:
cat: /etc/tomcat/context.xml: No such file or directory

This way, I can make sure that mapping is working correctly. Am I correct?

@aviramha
Copy link
Member

aviramha commented Nov 6, 2024

No - the path replacement happens in the libc level, meaning that your application won't even know that the path was replaced - hence it'd think it actually read from the original path.

@aviramha aviramha removed the bug Something isn't working label Nov 6, 2024
@milad2golnia
Copy link
Author

milad2golnia commented Nov 6, 2024

Ah, OK. I didn't imagine such low level replacement👌

I tested some patterns recently:


    "fs": {
      "mode": "write",

      "mapping": {
        "^/app/media": "/app",
        "^/app/pkgs": "/app",
        "^/app/notexistfile": "/app",
        "^/mnt": "/app",
        "^/somerandom": "/app",
        "^/dev": "/app",
        "^/srv": "/app",
        "^/etc": "/app"
      }
    },

All of them works successfully except followings:

  1. /somerandom
  2. /app/notexistfile

These two files doesn't exists on pod but all other files exist on pod. So I conclude that replacement occurs only after checking for existence.


Do you have /etc/tomcat/context.xml in the remote pod?

By this assumption, besides existing /etc/tomcat/context.xml, the original path (/home/johndoe/dev/tomcat/context.xml) should exists on pod too. And that's why my initial command (mentioned in issue description) fails.

To make sure this behavior, I performed following test too:

  1. I created following file in pod:
mkdir /app/dummy/
  1. I configured mirrord mapping as follow:

    "fs": {
      "mode": "write",

      "mapping": {
        "^/app/dummy": "/app",
      }
    },
  1. Ran application to execute ls /app/dummy and it succeeded.
  2. Remove dummy directory: rmdir /app/dummy/.
  3. Stop mirrord and ran debug again.
  4. It fails with /app/dummy: No such file or directory

If this is expected behavior of fs.feature.mapping I suggest to add these conditions to documentation (Or I'll do it myself when I had time upon your confirmation). But if you expect to mapping occurs regardless of source path existence, then this is a bug and I can create another issue which points out this bug separately.

@aviramha
Copy link
Member

aviramha commented Nov 6, 2024

The mapping feature happens before we decide whether to do a local access or a remote access.
Can you elaborate on the last part of your message explaining what you expected to happen that didn't happen and what code you ran using mirrord?

@milad2golnia
Copy link
Author

milad2golnia commented Nov 6, 2024

The mapping feature happens before we decide whether to do a local access or a remote access.

I expected that too and I prefer to see mappings applied before all other operations and checkings.

OK. I'll give you full overview over the problem.

Repeat following steps to reproduce the problem:

Correct Workflow

  1. First of all, let's look at filesystem on pod.
$ ls /
app  bin  boot  dev  etc  home  lib  lib64  media  mnt  opt  proc  root  run  sbin  somerandom  srv  sys  tmp  usr  var
ls /app/
dist  dummy  media  node_modules  null  pkgs
  1. Now, let's create a new file on pod:
$ mkdir /test-mirrord-mapping
$ ls /
app  bin  boot  dev  etc  home  lib  lib64  media  mnt  opt  proc  root  run  sbin  somerandom  srv  sys  test-mirrord-mapping  tmp  usr  var
  1. Here is mirrord configuration:
{
  "target": {
    "path": {
      "deployment": "my-pod"
    },
    "namespace": "services"
  },
  "feature": {
    "env": {
      "override": {
        "CATEGORY_ID": "1",
      }
    },
    "fs": {
      "mode": "write",

      "mapping": {
        "/test-mirrord-mapping": "/app/"
      }
    },
    "network": true
  },
  "agent": {
    "namespace": "services",
    "startup_timeout": 360
  },
  "kubeconfig": "~/.kube/test",
  "kube_context": "test"
}

Here is my code:

  const child = spawn('ls', ['/test-mirrord-mapping'])
  child.on('error', (err) => {
    console.log(err)
  })
  child.stderr.on('data', (chunk) => {
    console.log('stderr:', chunk)
  })
  child.stdout.on('data', (chunk) => {
    console.warn('stdout:', chunk)
  })
  1. Let's run the code and see output:
stdout:
dist dummy media node_modules null pkgs

Beautiful! It's working correctly.

Problem

Now I continue to reproduce the bug.
Continue following steps:

  1. Remove created directory from pod:
$ ls /
app  bin  boot  dev  etc  home  lib  lib64  media  mnt  opt  proc  root  run  sbin  somerandom  srv  sys  test-mirrord-mapping  tmp  usr  var
$ rmdir /test-mirrord-mapping
$ ls /
$ app  bin  boot  dev  etc  home  lib  lib64  media  mnt  opt  proc  root  run  sbin  somerandom  srv  sys  tmp  usr  var
  1. Now, Back to local machine, Don't touch mirrord configuration and code! Just stop debugger and start it again. Let's see what happens:
stderr:
ls: cannot access '/test-mirrord-mapping': No such file or directory

And that is the bug.

What is expected behavior? I expect to see following output again:

stdout:
dist dummy media node_modules null pkgs

More information

Here is my .vscode/launch.json in case you are curious:

{
    // Use IntelliSense to learn about possible attributes.
    // Hover to view descriptions of existing attributes.
    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
    "version": "0.2.0",
    "configurations": []
}

Please let me know if you need any other information

@aviramha
Copy link
Member

aviramha commented Nov 6, 2024

Thank you! I've prioritized it - will update once we get working on it.

@milad2golnia
Copy link
Author

Hi @aviramha,

I’ve encountered another issue related to mapping. I decided to mention it here rather than opening a new issue, as I believe both issues may have the same root cause. However, if you feel this should be treated as a separate problem, please let me know, and I’ll open a new issue for it.

The problem is that the mapping feature doesn’t seem to support relative paths. If mapping truly happens before any other operations, then we should be able to map relative paths as well.

This would be extremely useful, especially when debugging a program that uses relative paths—I can’t convert all relative paths to absolute paths easily.

I’ll try to clarify the issue further below.

Absolute Paths Mapping

  1. First of all, We take a look at pod file system and create a directory:
$ ls /
app  bin  boot  dev  etc  home  lib  lib64  media  mnt  opt  proc  root  run  sbin  srv  sys  tmp  usr  var
$ ls /app
dist  dummy  media  node_modules  null  pkgs
$ mkdir /dummy
$ ls /
app  bin  boot  dev  dummy  etc  home  lib  lib64  media  mnt  opt  proc  root  run  sbin  srv  sys  tmp  usr  var
  1. OK, now we are ready to test mapping. Here is mirrord configuration:
{
  "target": {
    "path": {
      "deployment": "my-pod"
    },
    "namespace": "services"
  },
  "feature": {
    "env": {
      "override": {
        "CATEGORY_ID": "1"
      }
    },
    "fs": {
      "mode": "write",

      "mapping": {
        ".+dummy$": "/app"
      }
    },
    "network": true
  },
  "agent": {
    "namespace": "services",
    "startup_timeout": 360
  },
  "kubeconfig": "~/.kube/test",
  "kube_context": "test"
}

  1. The code to test absolute path mapping:
  const child = spawn('ls', ['/dummy'])
  child.on('error', (err) => {
    console.log(err)
  })
  child.stderr.on('data', (chunk) => {
    console.log('stderr:', chunk)
  })
  child.stdout.on('data', (chunk) => {
    console.warn('stdout:', chunk)
  })

Let's see output:

stdout:
 dist dummy media node_modules null pkgs

Amazing! It's working.

Problem

Here we continue to produce problem.

  1. Don't change mirrord configuration and pod's file system. Just change the code to use relative path instead of absolute path:
const child = spawn('ls', ['../dummy'])
  child.on('error', (err) => {
    console.log(err)
  })
  child.stderr.on('data', (chunk) => {
    console.log('stderr:', chunk)
  })
  child.stdout.on('data', (chunk) => {
    console.warn('stdout:', chunk)
  })

And here is output:

stderr:
ls: cannot access '../dummy': No such file or directory

Which indicates mapping didn't work.

What is expected behavior? Since ../dummy should get matched against .+dummy$ pattern, I'll expect to see content of /app in output:


stdout:
 dist dummy media node_modules null pkgs

Let me know if you need more information.

@aviramha
Copy link
Member

aviramha commented Nov 8, 2024

Hey,
We found an issue with file mapping, fixed in latest release - can you try it out please?

@milad2golnia
Copy link
Author

Hey,
I tried out mirrord 3.124.2 version and still both problems are presents:

  1. Source file should present in pod file system to map it.
  2. Mapping of relative paths doesn't work

In new version, it seems another problem is introduced too! Now I'm not able to map some paths like /etc, but it was previously fine.

@aviramha
Copy link
Member

aviramha commented Nov 9, 2024

Can you share the more information about the regression?

@aviramha
Copy link
Member

aviramha commented Nov 9, 2024

BTW regarding relative mapping - that's not supported - that's a known issue that we haven't prioritised yet. (in general, we don't apply any fs logic to relative paths, since it's trickier than absolute paths.)

@milad2golnia
Copy link
Author

Can you share the more information about the regression?

The reproduction steps are the same as before, so please refer to this message for a complete overview of problem. for a complete overview of the problem.

BTW regarding relative mapping - that's not supported - that's a known issue that we haven't prioritised yet. (in general, we don't apply any fs logic to relative paths, since it's trickier than absolute paths.)

I completely understand your reasoning, and it makes sense. I’d like to suggest adding a note in the relevant documentation about the limitations of mapping relative paths. This could help users avoid potential confusion when working with this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants