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

test: update a read-only setting #1547

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

test: update a read-only setting #1547

wants to merge 1 commit into from
+96 −4

Conversation

mantissahz
Copy link
Contributor

@mantissahz mantissahz self-assigned this Oct 5, 2023
@mantissahz mantissahz force-pushed the issue5989 branch 2 times, most recently from b60abc4 to 710154e Compare October 5, 2023 02:03
@mantissahz mantissahz mentioned this pull request Oct 5, 2023
Closed
@mantissahz mantissahz marked this pull request as ready for review October 27, 2023 01:33
@mantissahz mantissahz requested a review from a team as a code owner October 27, 2023 01:33
khushboo-rancher
khushboo-rancher requested changes Nov 1, 2023
View reviewed changes
Copy link
Collaborator

@khushboo-rancher khushboo-rancher left a comment

Choose a reason for hiding this comment

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

Test steps look fine, we can consider a small change.

manager/integration/tests/test_settings.py Show resolved Hide resolved
@yangchiu
Copy link
Member

@mantissahz Could you help to confirm if this test case can pass on master-head? Because I tried to run this test case, but it failed with assertion error: https://ci.longhorn.io/job/private/job/longhorn-tests-regression/5308/testReport/junit/tests/test_settings/test_setting_update_readonly/

Thanks!

@mantissahz
Copy link
Contributor Author

@mantissahz Could you help to confirm if this test case can pass on master-head? Because I tried to run this test case, but it failed with an assertion error: https://ci.longhorn.io/job/private/job/longhorn-tests-regression/5308/testReport/junit/tests/test_settings/test_setting_update_readonly/

Sure, and no, it would not pass now because we still allow modifying read-only settings by APIs.
Do we have any method to update the setting by the manifest in our regression test?

@yangchiu
Copy link
Member

Sure, and no, it would not pass now because we still allow modifying read-only settings by APIs. Do we have any method to update the setting by the manifest in our regression test?

Yes, kubectl is available in the test, Does calling kubectl patch to update the setting solve the problem?

@mantissahz
Copy link
Contributor Author

Does calling kubectl patch to update the setting solve the problem?

@yangchiu,
Yes, it should solve the problem by the kubectl patch cmd.

@mergify Mergify
Copy link

mergify bot commented Dec 29, 2023

This pull request is now in conflicts. Could you fix it @mantissahz? 🙏

1 similar comment
@mergify Mergify
Copy link

mergify bot commented Feb 6, 2024

This pull request is now in conflicts. Could you fix it @mantissahz? 🙏

@mantissahz
test: update a read-only setting
3a20152 
Ref: 5989

Signed-off-by: James Lu <jamesluhz@gmail.com>
Signed-off-by: James Lu <james.lu@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@khushboo-rancher khushboo-rancher Awaiting requested review from khushboo-rancher

@yangchiu yangchiu Awaiting requested review from yangchiu

Requested changes must be addressed to merge this pull request.

Assignees

@mantissahz mantissahz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mantissahz @yangchiu @khushboo-rancher