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

Cherry-pick PR #354, #355, #360, and #398 to release-v1.2 #418

Merged
merged 4 commits into from
May 2, 2024
Merged

Cherry-pick PR #354, #355, #360, and #398 to release-v1.2 #418

merged 4 commits into from
May 2, 2024

Conversation

thunderboltsid
Copy link
Contributor

Cherry-Pick Details

@thunderboltsid thunderboltsid requested a review from dkoshkin April 24, 2024 21:03
This was referenced Apr 25, 2024
Closed
Closed
adiantum
adiantum approved these changes Apr 30, 2024
View reviewed changes
Copy link
Contributor

@adiantum adiantum left a comment

Choose a reason for hiding this comment

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

/lgtm

faiq and others added 4 commits May 2, 2024 15:04
@faiq @thunderboltsid
fix: improve error handling for Prism Client (#354)
0a213e7 
* fix: use wrapper errors to clearly denote issues in client building

* fix: adds a function to properly sanitize the address

* fix: adds tests for ip address case given

* fix: uses a defined type for port error

* fix: clean up variable naming

* fix: remove validation here to be moved into prism-client
@dkoshkin @thunderboltsid
refactor: task status file (#355)
0c8cd85 
* test: add unit tests for pkg/client/state

* refactor: use wait.Poll function waiting for task state

* refactor: use consistent task status names

* fixup! test: add unit tests for pkg/client/state

* fix: revert to previous behaviod polling forever

The ctx passed into WaitForTaskToSucceed is only used to cancel HTTP reqests and not to cancel the wait.

* chore: add license headers

* fix: better function name
@dkoshkin @thunderboltsid
refactor: client.go file helper methods (#360)
adfccda 
* refactor: client.go file helper methods

Refactored the existing methods and functions to be unit testable.
Also made some methods that do not use the struct as generic functions.
The changes were primarily an effort to add unit test coverage.

* refactor: more testable read file function

* test: new nutanixcluster types unit tests

* test: additional test cases for errors

* fixup! refactor: client.go file helper methods

* fixup! refactor: client.go file helper methods

* fixup! refactor: more testable read file function

* fixup! refactor: client.go file helper methods
@thunderboltsid
Switch Nutanix Client to using Session Auth (#398)
786115e 
This will ensure we make fewer basic auth requests to Prism Central
IAM Services.
@github-actions GitHub Actions
Copy link

github-actions bot commented May 2, 2024

✅ None of your dependencies violate policy!

@codecov Codecov
Copy link

codecov bot commented May 2, 2024

Codecov Report

Attention: Patch coverage is 79.54545% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 9.28%. Comparing base (aa87a8f) to head (786115e).

Files Patch % Lines
pkg/client/client.go 84.21% 9 Missing ⚠️
controllers/nutanixcluster_controller.go 0.00% 5 Missing ⚠️
controllers/helpers.go 0.00% 3 Missing ⚠️
controllers/nutanixmachine_controller.go 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@               Coverage Diff               @@
##           release-v1.2    #418      +/-   ##
===============================================
+ Coverage          2.61%   9.28%   +6.67%     
===============================================
  Files                 4      10       +6     
  Lines               995    1335     +340     
===============================================
+ Hits                 26     124      +98     
- Misses              969    1211     +242

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@thunderboltsid
Copy link
Contributor Author

/test e2e-k8s-conformance
/test e2e-k8s-upgrade
/test e2e-nutanix-features

@nutanix-cn-prow-bot
Copy link

nutanix-cn-prow-bot bot commented May 2, 2024 •
edited
Loading

@thunderboltsid: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-capx-controller-upgrade 9c7ba34 link false /test e2e-capx-controller-upgrade
ci/prow/e2e-nutanix-features 786115e link false /test e2e-nutanix-features

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@thunderboltsid
Copy link
Contributor Author

thunderboltsid commented May 2, 2024 •
edited
Loading

@thunderboltsid: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-capx-controller-upgrade 9c7ba34 link false /test e2e-capx-controller-upgrade
ci/prow/e2e-k8s-upgrade 9c7ba34 link false /test e2e-k8s-upgrade
ci/prow/e2e-nutanix-features 786115e link false /test e2e-nutanix-features
Full PR test history. Your PR dashboard.

e2e-nutanix-features test suite actually passed but it seems the !cluster-upgrade-conformance && !conformance as described in prow job

            - name: LABEL_FILTERS
              value: "!cluster-upgrade-conformance && !conformance"

did not get passed to the the test command. As a result, it took longer for the test to run and it only skipped cluster-upgrade-conformance.

NUTANIX_LOG_LEVEL=debug /home/prow/go/src/github.com/nutanix-cloud-native/cluster-api-provider-nutanix/hack/tools/bin/ginkgo-v2.1.4 -v \
	--trace \
	--progress \
	--tags=e2e \
	--label-filter="!only-for-validation && !cluster-upgrade-conformance" \
	--skip=""clusterctl-Upgrade"" \
	--focus="""" \
	--nodes=1 \
	--no-color=false \
	--output-dir="/logs/artifacts" \
	--junit-report="junit.e2e_suite.1.xml" \
	--timeout="24h" \
	--always-emit-ginkgo-writer \
	 ./test/e2e -- \
	-e2e.artifacts-folder="/logs/artifacts" \
	-e2e.config="/home/prow/go/src/github.com/nutanix-cloud-native/cluster-api-provider-nutanix/test/e2e/config/nutanix.yaml" \
	-e2e.skip-resource-cleanup=false \
	-e2e.use-existing-cluster=false
...
...
...
Ginkgo ran 1 suite in 1h19m1.425626558s
Test Suite Passed
make[1]: Leaving directory '/home/prow/go/src/github.com/nutanix-cloud-native/cluster-api-provider-nutanix'
make: *** No rule to make target '&&'.  Stop.

@thunderboltsid
Copy link
Contributor Author

Testing out a potential fix with with 79c6cd1

@thunderboltsid thunderboltsid merged commit 6cb4938 into release-v1.2 May 2, 2024
9 of 11 checks passed
@thunderboltsid thunderboltsid deleted the jira/krbn-8096 branch May 2, 2024 16:45
@thunderboltsid thunderboltsid mentioned this pull request May 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepakm-ntnx deepakm-ntnx deepakm-ntnx approved these changes

@adiantum adiantum adiantum approved these changes

@dkoshkin dkoshkin Awaiting requested review from dkoshkin

Assignees

@adiantum adiantum

Labels
do-not-merge/hold lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@thunderboltsid @adiantum @deepakm-ntnx @faiq @dkoshkin