-
Notifications
You must be signed in to change notification settings - Fork 356
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
feat:sealer local e2e test and CI using github action #1922
Conversation
Codecov ReportBase: 17.47% // Head: 17.47% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #1922 +/- ##
=======================================
Coverage 17.47% 17.47%
=======================================
Files 103 103
Lines 9169 9169
=======================================
Hits 1602 1602
Misses 7355 7355
Partials 212 212
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
What's the relationship between this PR and https://github.com/sealerio/sealer/pull/1893? BTW, please solve the conflict firstly |
|
2d5379b
to
625e5dc
Compare
625e5dc
to
aafc87c
Compare
9d5165e
to
6851c41
Compare
@@ -25,5 +25,5 @@ func CommandSetHostAlias(hostName, ip string) string { | |||
} | |||
|
|||
func CommandUnSetHostAlias() string { | |||
return fmt.Sprintf(`sed -i "/%s/d" /etc/hosts`, DefaultSealerHostAliasAnnotation) | |||
return fmt.Sprintf(`echo "$(sed "/%s/d" /etc/hosts)" > /etc/hosts`, DefaultSealerHostAliasAnnotation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a dangerous operation directly overwrite the entire /etc/hosts file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file /etc/hosts
is mounted by docker, which means you are not allowed to change its inode from within docker container, so it will cause error Device or resource busy
(see:find-and-sed-string-in-docker-got-error-device-or-resource-busy), when using sealer delete
command which will use sed
to change /etc/hosts
.In fact, the essence of the sed
command is not to modify the file, but to create a new file to replace the existing file(just like directly overwrite file?)(see:docker中执行sed), and you can see after sed
the inode of the file will be changed which is not allowed in container environment with /etc/hosts
.
1b0908f
to
5a11d61
Compare
Signed-off-by: zhy76 <958474674@qq.com>
5a11d61
to
1b2d6b1
Compare
PTAL |
err := testhelper.MarshalYamlToFile(clusterFilePath, cluster) | ||
//convert clusterfilev1 to clusterfilev2 | ||
clusterv2 := utils.ConvertV1ClusterToV2Cluster(cluster) | ||
clusterv2.Spec.Env = []string{"env=TestEnv"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extract env=TestEnv
as a common env?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@kakaZhou719 PTAL again. THX |
Signed-off-by: zhy76 958474674@qq.com
Describe what this PR does / why we need it
feature:local e2e test and CI using github action
Does this pull request fix one issue?
#1633 #1734
Describe how you did it
Infra using docker and run containers as nodes for sealer e2e test,using github action for sealer CI.
Describe how to verify it
See /test/README.md
Special notes for reviews
Before we use the CI using github aciton feature, please make sure that these three variables: REGISTRY_URL={your registry},
REGISTRY_USERNAME={user name},REGISTRY_PASSWORD={password} have been setted in github's secret (for sealer login and sealer image tests).