Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

add watchconsole for no_proxy type#1933

Merged
bergwolf merged 1 commit intokata-containers:masterfrom
lifupan:noproxywatchconsole
Aug 16, 2019
Merged

add watchconsole for no_proxy type#1933
bergwolf merged 1 commit intokata-containers:masterfrom
lifupan:noproxywatchconsole

Conversation

@lifupan
Copy link
Copy Markdown
Member

@lifupan lifupan commented Aug 6, 2019

For no proxy type, we also need the feature
of watch hypervisor's console to help debug.

Fixes:#1932

Signed-off-by: lifupan lifupan@gmail.com

@lifupan lifupan force-pushed the noproxywatchconsole branch 5 times, most recently from 726a1fd to 933053e Compare August 6, 2019 08:07
@lifupan
Copy link
Copy Markdown
Member Author

lifupan commented Aug 6, 2019

/test

@grahamwhaley grahamwhaley requested review from bergwolf and devimc August 6, 2019 08:48
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 6, 2019

Codecov Report

Merging #1933 into master will decrease coverage by 0.38%.
The diff coverage is 44.23%.

@@            Coverage Diff             @@
##           master    #1933      +/-   ##
==========================================
- Coverage   52.16%   51.78%   -0.39%     
==========================================
  Files         108      107       -1     
  Lines       14227    14535     +308     
==========================================
+ Hits         7422     7527     +105     
- Misses       5923     6124     +201     
- Partials      882      884       +2

Copy link
Copy Markdown

@devimc devimc left a comment

Choose a reason for hiding this comment

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

@lifupan thanks, good work!

if err != nil {
return err
}
// TODO: add pty console support for kvmtools
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

could you please file an issue and mention it here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi @devimc Done.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

thanks

@lifupan lifupan force-pushed the noproxywatchconsole branch from 933053e to e9beb5e Compare August 7, 2019 03:12
@devimc
Copy link
Copy Markdown

devimc commented Aug 7, 2019

/test

return -1, "", fmt.Errorf("kata proxy running for sandbox %s", params.id)
}

params.logger.Debug("Starting kata proxy")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@lifupan This will be logged for the the noProxy.start() as well which is not desired.
Similarly I would suggest changing the above Error message as well to something like "Error setting up console" for the noProxy case, else the user may think that an actual proxy is running looking at the logs/errors.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi @amshinde, Updated!

For no proxy type, we also need the feature
of watch hypervisor's console to help debug.

Fixes:kata-containers#1932

Signed-off-by: lifupan <lifupan@gmail.com>
@lifupan lifupan force-pushed the noproxywatchconsole branch from e9beb5e to 31ddb4d Compare August 13, 2019 01:11
@lifupan
Copy link
Copy Markdown
Member Author

lifupan commented Aug 13, 2019

/test

@devimc
Copy link
Copy Markdown

devimc commented Aug 13, 2019

restarting ubuntu and firecracker CI

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

For no_proxy type, it also needs to watch hypervisor's console

5 participants