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.

Sign up for GitHub

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

Jump to bottom

Add Rotation certification Check, remove func to restart agents #7097

Merged
merged 7 commits into from
May 12, 2023
Merged

Add Rotation certification Check, remove func to restart agents #7097

merged 7 commits into from
May 12, 2023

Conversation

est-suse
Copy link
Contributor

Proposed Changes

Types of Changes

Test

Verification

Run the test

Testing

N/A

Linked Issues

rancher/qa-tasks#597

User-Facing Change

None

Further Comments

@est-suse est-suse requested a review from a team as a code owner March 15, 2023 16:28
@est-suse est-suse requested a review from a team March 15, 2023 16:28
dereknola
dereknola requested changes Mar 17, 2023
View reviewed changes
Copy link
Member

@dereknola dereknola left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This testing should take place in a new, separate, E2E test. We do not want to add to validate_cluster.

@est-suse est-suse closed this Mar 24, 2023
Create test for certificate rotation, delete restartAgent Function an…
0bccd71 
…d add functions for start and stop server

Signed-off-by: est-suse <esteban.esquivel@suse.com>
@est-suse est-suse reopened this Apr 3, 2023
Delete Additional comments in the Test file
55168fb 
Signed-off-by: est-suse <esteban.esquivel@suse.com>
@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2023 •
edited by codecov bot
Loading

Codecov Report

Patch coverage has no change and project coverage change: +10.17 🎉

Comparison is base (a993766) 9.66% compared to head (9b9df2f) 19.83%.

Additional details and impacted files 
@@             Coverage Diff             @@
##           master    #7097       +/-   ##
===========================================
+ Coverage    9.66%   19.83%   +10.17%     
===========================================
  Files         147       81       -66     
  Lines       10766     7621     -3145     
===========================================
+ Hits         1040     1512      +472     
+ Misses       9508     5882     -3626     
- Partials      218      227        +9
Flag Coverage Δ
unittests 19.83% <ø> (+10.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 147 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

fmoral2
fmoral2 reviewed Apr 3, 2023
View reviewed changes
tests/e2e/testutils.go Outdated Show resolved Hide resolved
Correct different issues, formats, move certifcate rotation func
f3ebe2f 
Signed-off-by: est-suse <esteban.esquivel@suse.com>
@est-suse est-suse requested review from fmoral2, dereknola and a team April 3, 2023 19:34
est-suse added 2 commits April 5, 2023 10:22
Fix limit lenght, remove Eventually Block and create a new file for R…
b3eaef9 
…otateCertificate command

Signed-off-by: est-suse <esteban.esquivel@suse.com>
Move Function Rotate Certificate to Utils file
e1e98bc 
Signed-off-by: est-suse <esteban.esquivel@suse.com>
@dereknola
Copy link
Member

There is not longer a need to add an additional certificate rotation E2E test. One already exists at https://github.com/k3s-io/k3s/tree/master/tests/e2e/rotateca

ShylajaDevadiga
ShylajaDevadiga reviewed May 4, 2023
View reviewed changes
tests/e2e/rotatecertificate/rotatecertificate_test.go Outdated
Expect(err).NotTo(HaveOccurred())
})

It("Verifies Certificate Rotation", func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add this test as part of validate cluster? I suggest we don't need to create new resource for this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has been moved.
Thanks

@est-suse est-suse force-pushed the master branch 2 times, most recently from 1c16f5b to 7df8f15 Compare May 5, 2023 19:25
rancher-max
rancher-max requested changes May 5, 2023
View reviewed changes
Copy link
Contributor

@rancher-max rancher-max left a comment

Choose a reason for hiding this comment

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

Just remove some of the extra printlns that are in here and then it LGTM

@est-suse est-suse requested a review from rancher-max May 5, 2023 21:29
ShylajaDevadiga
ShylajaDevadiga reviewed May 8, 2023
View reviewed changes
tests/e2e/validatecluster/validatecluster_test.go Outdated
@@ -203,13 +204,16 @@ var _ = Describe("Verify Create", Ordered, func() {

Eventually(func(g Gomega) {
cmd := "kubectl get pods dnsutils --kubeconfig=" + kubeConfigFile
fmt.Println("ESTEBAN DNS ACESS", cmd)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove debug lines

@est-suse est-suse force-pushed the master branch 3 times, most recently from 27c20b3 to c2da4f1 Compare May 9, 2023 19:22
Move the Certification Test to Validate Cluster
adde679 
Signed-off-by: est-suse <esteban.esquivel@suse.com>
@dereknola
Fix to stop/start for k3s certificate rotation
9b9df2f 
Signed-off-by: Derek Nola <derek.nola@suse.com>
@dereknola dereknola merged commit 9bcfac8 into k3s-io:master May 12, 2023
@est-suse est-suse mentioned this pull request May 12, 2023
Merged
@cguertin14 cguertin14 mentioned this pull request May 31, 2023
Merged
dereknola added a commit to dereknola/k3s that referenced this pull request Jun 8, 2023
@est-suse @dereknola
Add Rotation certification Check (k3s-io#7097)
57fda5e 
* Add Certification Test to Validate Cluster

Signed-off-by: est-suse <esteban.esquivel@suse.com>

* Fix to stop/start for k3s certificate rotation

Signed-off-by: Derek Nola <derek.nola@suse.com>

---------

Signed-off-by: est-suse <esteban.esquivel@suse.com>
Signed-off-by: Derek Nola <derek.nola@suse.com>
Co-authored-by: est-suse <esteban.esquivel@suse.com>
Co-authored-by: Derek Nola <derek.nola@suse.com>
@dereknola dereknola mentioned this pull request Jun 8, 2023
Merged
dereknola added a commit to dereknola/k3s that referenced this pull request Jun 8, 2023
@est-suse @dereknola
Add Rotation certification Check (k3s-io#7097)
dd1bf5b 
* Add Certification Test to Validate Cluster

Signed-off-by: est-suse <esteban.esquivel@suse.com>

* Fix to stop/start for k3s certificate rotation

Signed-off-by: Derek Nola <derek.nola@suse.com>

---------

Signed-off-by: est-suse <esteban.esquivel@suse.com>
Signed-off-by: Derek Nola <derek.nola@suse.com>
Co-authored-by: est-suse <esteban.esquivel@suse.com>
Co-authored-by: Derek Nola <derek.nola@suse.com>
dereknola added a commit that referenced this pull request Jun 8, 2023
@dereknola @est-suse @brandond
[Release-1.26] E2E and Dep Backports - June (#7693)
8988439 
* Bump docker go.mod (#7681)
* Shortcircuit commands with version or help flags (#7683)
* Fix for longhorn integration test
* Add Rotation certification Check (#7097)
* Add Certification Test to Validate Cluster
* Fix to stop/start for k3s certificate rotation
* E2E: Use sudo for all RunCmdOnNode
* Remove unnecessary daemonset addition/deletion

Signed-off-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: est-suse <esteban.esquivel@suse.com>
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Co-authored-by: Esteban Esquivel Alvarado <esteban.alvarado@suse.com>
Co-authored-by: est-suse <esteban.esquivel@suse.com>
Co-authored-by: Brad Davidson <brad.davidson@rancher.com>
@dereknola dereknola mentioned this pull request Jun 8, 2023
Merged
dereknola added a commit that referenced this pull request Jun 9, 2023
@est-suse @dereknola
Add Rotation certification Check (#7097)
ed6e7b1 
* Add Certification Test to Validate Cluster

Signed-off-by: est-suse <esteban.esquivel@suse.com>

* Fix to stop/start for k3s certificate rotation

Signed-off-by: Derek Nola <derek.nola@suse.com>

---------

Signed-off-by: est-suse <esteban.esquivel@suse.com>
Signed-off-by: Derek Nola <derek.nola@suse.com>
Co-authored-by: est-suse <esteban.esquivel@suse.com>
Co-authored-by: Derek Nola <derek.nola@suse.com>
dereknola added a commit to dereknola/k3s that referenced this pull request Jun 9, 2023
@est-suse @dereknola
Add Rotation certification Check (k3s-io#7097)
7883fdf 
* Add Certification Test to Validate Cluster

Signed-off-by: est-suse <esteban.esquivel@suse.com>

* Fix to stop/start for k3s certificate rotation

Signed-off-by: Derek Nola <derek.nola@suse.com>

---------

Signed-off-by: est-suse <esteban.esquivel@suse.com>
Signed-off-by: Derek Nola <derek.nola@suse.com>
Co-authored-by: est-suse <esteban.esquivel@suse.com>
Co-authored-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: Derek Nola <derek.nola@suse.com>
@dereknola dereknola mentioned this pull request Jun 9, 2023
Merged
dereknola added a commit that referenced this pull request Jun 10, 2023
@est-suse @dereknola
Add Rotation certification Check (#7097)
872789c 
* Add Certification Test to Validate Cluster

Signed-off-by: est-suse <esteban.esquivel@suse.com>

* Fix to stop/start for k3s certificate rotation

Signed-off-by: Derek Nola <derek.nola@suse.com>

---------

Signed-off-by: est-suse <esteban.esquivel@suse.com>
Signed-off-by: Derek Nola <derek.nola@suse.com>
Co-authored-by: est-suse <esteban.esquivel@suse.com>
Co-authored-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: Derek Nola <derek.nola@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShylajaDevadiga ShylajaDevadiga ShylajaDevadiga left review comments

@dereknola dereknola dereknola approved these changes

@rancher-max rancher-max rancher-max approved these changes

@fmoral2 fmoral2 Awaiting requested review from fmoral2

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@est-suse @codecov-commenter @dereknola @rancher-max @ShylajaDevadiga @fmoral2