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

Ensure exec command can find config.json #616

Merged
merged 1 commit into from
Jan 19, 2022
Merged

Conversation

Furisto
Copy link
Collaborator

@Furisto Furisto commented Jan 16, 2022

Partially fixes #584. While fixing this I discovered another issue, which I will follow up in the next PR.

@utam0k
Copy link
Member

utam0k commented Jan 17, 2022

@Furisto I'm sorry, but could you rebase?

@codecov-commenter
Copy link

Codecov Report

Merging #616 (31c30af) into main (63f77b2) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #616      +/-   ##
==========================================
- Coverage   70.01%   69.99%   -0.02%     
==========================================
  Files          82       82              
  Lines       10933    10936       +3     
==========================================
  Hits         7655     7655              
- Misses       3278     3281       +3     

@Furisto
Copy link
Collaborator Author

Furisto commented Jan 17, 2022

@utam0k PTAL

@utam0k
Copy link
Member

utam0k commented Jan 18, 2022

@Furisto Thanks for your PR 😍
Doesn't this break the exec command? The config.json should not be in the bundle directory. I tried the following command and it failed.

$ sudo podman run --name test -it busybox sh
/ 
# and in another console
$ sudo podman exec -it test sh

@Furisto
Copy link
Collaborator Author

Furisto commented Jan 18, 2022

@utam0k The exec command is already broken. This fixes one of the issues (missing config.json), but there is another issue which I alluded to the in the description of this issue which breaks the exec command later on.

The config.json should not be in the bundle directory.

Unless I am misunderstanding your meaning, it is in the bundle directory.

@utam0k
Copy link
Member

utam0k commented Jan 19, 2022

@utam0k The exec command is already broken. This fixes one of the issues (missing config.json), but there is another issue which I alluded to the in the description of this issue which breaks the exec command later on.

The config.json should not be in the bundle directory.

Unless I am misunderstanding your meaning, it is in the bundle directory.

Oh, my local was wrong.

Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@utam0k utam0k merged commit f5e5aeb into youki-dev:main Jan 19, 2022
@Furisto Furisto linked an issue Jan 21, 2022 that may be closed by this pull request
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

Successfully merging this pull request may close these issues.

Revert back to saving full config.json instead of youki config Using youki and cri-o to run Kubernetes
3 participants