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

ci: enable and fixes for nfs ci #10510

Merged
merged 1 commit into from
Oct 6, 2022
Merged

Conversation

Rakshith-R
Copy link
Member

@Rakshith-R Rakshith-R commented Jun 27, 2022

This commit anabled nfs csi ci and
add fixes/improvements to it like the
following:

  • verify deletion of cephnfs and .nfs pool before proceeding
  • verify pv deletion
  • do not enable rook module
  • reduce activeCount to 1 to save resources
  • run cephnfs ci before cephfs ci since it cephfs
    ci is more resource intensive.

Signed-off-by: Rakshith R rar@redhat.com

Resolves #

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide).
  • Skip Tests for Docs: If this is only a documentation change, add the label skip-ci on the PR.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@Rakshith-R Rakshith-R force-pushed the fix-nfs-ci branch 10 times, most recently from d25d022 to 591e3f3 Compare July 4, 2022 07:49
@Rakshith-R Rakshith-R force-pushed the fix-nfs-ci branch 9 times, most recently from 8734cc4 to 01bb73e Compare July 5, 2022 13:54
@mergify
Copy link

mergify bot commented Jul 5, 2022

This pull request has merge conflicts that must be resolved before it can be merged. @Rakshith-R please rebase it. https://rook.io/docs/rook/latest/Contributing/development-flow/#updating-your-fork

@Rakshith-R Rakshith-R force-pushed the fix-nfs-ci branch 4 times, most recently from 3ca2ebc to c0df9f3 Compare July 6, 2022 10:12
@mergify
Copy link

mergify bot commented Jul 25, 2022

This pull request has merge conflicts that must be resolved before it can be merged. @Rakshith-R please rebase it. https://rook.io/docs/rook/latest/Contributing/development-flow/#updating-your-fork

1 similar comment
@mergify
Copy link

mergify bot commented Aug 11, 2022

This pull request has merge conflicts that must be resolved before it can be merged. @Rakshith-R please rebase it. https://rook.io/docs/rook/latest/Contributing/development-flow/#updating-your-fork

@Rakshith-R Rakshith-R force-pushed the fix-nfs-ci branch 2 times, most recently from 4484071 to d7fe636 Compare August 25, 2022 10:34
@Rakshith-R Rakshith-R force-pushed the fix-nfs-ci branch 8 times, most recently from 582d43c to 3d2f189 Compare September 3, 2022 11:30
@mergify
Copy link

mergify bot commented Sep 13, 2022

This pull request has merge conflicts that must be resolved before it can be merged. @Rakshith-R please rebase it. https://rook.io/docs/rook/latest/Contributing/development-flow/#updating-your-fork

@Rakshith-R Rakshith-R force-pushed the fix-nfs-ci branch 2 times, most recently from 9771c4c to dfee738 Compare September 26, 2022 06:05
@Rakshith-R Rakshith-R changed the title ci: verify deletion of cephnfs and .nfs pool before proceeding ci: enable and fixes for nfs ci Sep 26, 2022
@Rakshith-R Rakshith-R linked an issue Sep 26, 2022 that may be closed by this pull request
@Rakshith-R Rakshith-R marked this pull request as ready for review September 26, 2022 10:40
@@ -153,8 +153,6 @@ spec:
modules:
- name: pg_autoscaler
enabled: true
- name: rook
Copy link
Member Author

Choose a reason for hiding this comment

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

This was the reason why nfs pvc creation was failing.

Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

LGTM, before I approve if @BlaineEXE could take a look.

@@ -272,7 +272,7 @@ func (k8sh *K8sHelper) WaitForCustomResourceDeletion(namespace, name string, che
return err
}
logger.Errorf("gave up deleting custom resource %q ", name)
return nil
return fmt.Errorf("Timedout waiting for deletion of customer resource %q", name)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("Timedout waiting for deletion of customer resource %q", name)
return fmt.Errorf("Timed out waiting for deletion of custom resource %q", name)

if err != nil && kerrors.IsNotFound(err) {
if kerrors.IsNotFound(err) {
Copy link
Member

Choose a reason for hiding this comment

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

I think the err != nil part is important to make sure this doesn't segfault. Why was this change made?

Copy link
Member Author

Choose a reason for hiding this comment

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

done reverted it back.
I don't remember why this was changed.

@Rakshith-R
Copy link
Member Author

@Mergifyio rebase

This commit anabled nfs csi ci and
add fixes/improvements to it like the
following:
- verify deletion of cephnfs and .nfs pool before proceeding
- verify pv deletion
- do not enable rook module
- reduce activeCount to 1 to save resources
- run cephnfs ci before cephfs ci since it cephfs
  ci is more resource intensive.

Signed-off-by: Rakshith R <rar@redhat.com>
@mergify
Copy link

mergify bot commented Oct 6, 2022

rebase

✅ Branch has been successfully rebased

@BlaineEXE BlaineEXE merged commit a809de2 into rook:master Oct 6, 2022
mergify bot added a commit that referenced this pull request Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NFS CI test is disabled
3 participants