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

kata-deploy: Check crio conf before update #334

Merged
merged 1 commit into from
Feb 19, 2019

Conversation

ganeshmaharaj
Copy link
Contributor

kata-deploy inserts 'manage_network_ns_lifecycle' into crio.conf without any
prior checks and if there is a previous entry in the file, this becomes a
duplicate causing crio service restart issues. This patch addresses that
particular scenario.

Signed-off-by: Ganesh Maharaj Mahalingam ganesh.mahalingam@intel.com

@ganeshmaharaj
Copy link
Contributor Author

I am fine eitherways. Don't have much of a preference. Anyone from the core team have any preference? Am leaning towards what @krsna1729 is suggesting also cause of less escapes.

@jodh-intel
Copy link
Contributor

I vote for the simpler option with less escapes - it's clearer, so easier to understand and maintain.

@grahamwhaley
Copy link
Contributor

simpler, cleaner is always better :-)
I will note, throughout the whole of kata we tend to use $(xyz) rather than `xyz`. If you'd not mind changing that?

kata-deploy inserts 'manage_network_ns_lifecycle' into crio.conf without any
prior checks and if there is a previous entry in the file, this becomes a
duplicate causing crio service restart issues. This patch addresses that
particular scenario.

Signed-off-by: Ganesh Maharaj Mahalingam <ganesh.mahalingam@intel.com>
@ganeshmaharaj
Copy link
Contributor Author

@jodh-intel @krsna1729 @grahamwhaley thanks for the inputs. new patch up for review.

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

lgtm

@grahamwhaley
Copy link
Contributor

/test
not sure if the CI tests this... let's spin it..

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.

None yet

6 participants