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

PWX-28826 & PWX-30496: Pre-flight check for DMthin #1021

Merged
merged 19 commits into from
Apr 27, 2023

Conversation

jrivera-px
Copy link
Contributor

@jrivera-px jrivera-px commented Apr 25, 2023

What this PR does / why we need it:
Refer to : #1014 & #1019 comments
Which issue(s) this PR fixes (optional)
Closes #
PWX-28826 & PWX-30496
Special notes for your reviewer:
Cherry-picks from master - Need to ask Nikolay to do a cloud run before merging this in. - I believe these changes need to go in the 23.5.0 branch which does not exist yet. But please look at the changes anyhow..

harsh-px and others added 15 commits April 21, 2023 23:40
Signed-off-by: Harsh Desai <hadesai@purestorage.com>
Syncs DaemonSet changes done for auto- ssl/tls support:
* adding events/update RBAC permission
* adding certificatesigningrequests RBAC permissions
* adding containerdvardir (/var/lib/containerd) mount

Signed-off-by: Zoran Rajic <zrajic@purestorage.com>
Signed-off-by: Harsh Desai <hadesai@purestorage.com>
* PWX-28826 Boilerplace

Signed-off-by: Harsh Desai <hadesai@purestorage.com>

* more boilerplate

Signed-off-by: Harsh Desai <hadesai@purestorage.com>

* PWX-28826:  Pre-flight check for DMthin.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Add comments and move StorageNode cleanup.

Signed-off-by: Jose Rivera <jose@portworx.com>

* Passed checks should be Info events.

Signed-off-by: Jose Rivera <jose@portworx.com>

* Passed checks should be Info events. (#1010)

Signed-off-by: Jose Rivera <jose@portworx.com>

* Pwx 28826 (#1011)

* Pwx 28826 (#1012)

* PWX-28826: Update with the latest master changes. (#1013)

* Updating CSV to use 23.3.1 released image

* Update for 23.3.1 release

* Controller gen vendor

Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>

* PWX-29389 Add CRD for portworx diags collection

Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>

* PWX-29409: Ignore zones with no nodes (#1008)

In disaggregated mode, there could be zones in which no storage nodes
  might be present. Such a zone would make the maxSNPZ value to be 0.
  CHanging the behavior to ignore 0 nodes in a zone for maxSNPZ
  calculation.

Signed-off-by: Naveen Revanna <nrevanna@purestorage.com>

---------

Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>
Signed-off-by: Naveen Revanna <nrevanna@purestorage.com>
Co-authored-by: CNBU Jenkins <cnbu-jenkins@purestorage.com>
Co-authored-by: Jiafeng Liao <jliao@purestorage.com>
Co-authored-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>
Co-authored-by: Naveen Revanna <83608369+nrevanna@users.noreply.github.com>

* Add PassPreFlight event tag and logging

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Check status of portworx container in pre-flight pod and remove 'wait' code.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Fix unit test.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Fix unit test.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: PR review changes and fix portworx_test.go UTs

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: fix gomack Validate calls.  Also comment out the two tests that don't work since Validate was removed from the controller.validate() func. PWX-30373 to try and fix later.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-30373: Re-add back in the commented out tests and add K8s version check failure to trigger the needed workflow.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Exit pre-check wait if running CBT namespace.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Add 5 min timeout to pre-flight status check.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Exit GetPreFlightStatus() with success if running CBT namespace.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Don't automatically enable dmthin via pre-flight check if running CBT namespace.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-30373: Revert UT and integration test hacks.  Need to mock the functionality correctly.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Increase pre-flight daemonset ready wait to 10mins.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: fix 'TestValidate' UT.  Don't error if pre-flight daemonset exists.

Signed-off-by: Jose Rivera <jose@portworx.com>

* Only run preflight if AWS.

Signed-off-by: Jose Rivera <jose@portworx.com>

---------

Signed-off-by: Harsh Desai <hadesai@purestorage.com>
Signed-off-by: Jose Rivera <jose@portworx.com>
Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>
Signed-off-by: Naveen Revanna <nrevanna@purestorage.com>
Co-authored-by: Harsh Desai <hadesai@purestorage.com>
Co-authored-by: CNBU Jenkins <cnbu-jenkins@purestorage.com>
Co-authored-by: Jiafeng Liao <jliao@purestorage.com>
Co-authored-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>
Co-authored-by: Naveen Revanna <83608369+nrevanna@users.noreply.github.com>
…reflight fails don't start. (#1019)

* PWX-28826 Boilerplace

Signed-off-by: Harsh Desai <hadesai@purestorage.com>

* more boilerplate

Signed-off-by: Harsh Desai <hadesai@purestorage.com>

* PWX-28826:  Pre-flight check for DMthin.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Add comments and move StorageNode cleanup.

Signed-off-by: Jose Rivera <jose@portworx.com>

* Passed checks should be Info events.

Signed-off-by: Jose Rivera <jose@portworx.com>

* Passed checks should be Info events. (#1010)

Signed-off-by: Jose Rivera <jose@portworx.com>

* Pwx 28826 (#1011)

* Pwx 28826 (#1012)

* PWX-28826: Update with the latest master changes. (#1013)

* Updating CSV to use 23.3.1 released image

* Update for 23.3.1 release

* Controller gen vendor

Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>

* PWX-29389 Add CRD for portworx diags collection

Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>

* PWX-29409: Ignore zones with no nodes (#1008)

In disaggregated mode, there could be zones in which no storage nodes
  might be present. Such a zone would make the maxSNPZ value to be 0.
  CHanging the behavior to ignore 0 nodes in a zone for maxSNPZ
  calculation.

Signed-off-by: Naveen Revanna <nrevanna@purestorage.com>

---------

Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>
Signed-off-by: Naveen Revanna <nrevanna@purestorage.com>
Co-authored-by: CNBU Jenkins <cnbu-jenkins@purestorage.com>
Co-authored-by: Jiafeng Liao <jliao@purestorage.com>
Co-authored-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>
Co-authored-by: Naveen Revanna <83608369+nrevanna@users.noreply.github.com>

* Add PassPreFlight event tag and logging

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Check status of portworx container in pre-flight pod and remove 'wait' code.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Fix unit test.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Fix unit test.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: PR review changes and fix portworx_test.go UTs

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: fix gomack Validate calls.  Also comment out the two tests that don't work since Validate was removed from the controller.validate() func. PWX-30373 to try and fix later.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-30373: Re-add back in the commented out tests and add K8s version check failure to trigger the needed workflow.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Exit pre-check wait if running CBT namespace.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Add 5 min timeout to pre-flight status check.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Exit GetPreFlightStatus() with success if running CBT namespace.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Don't automatically enable dmthin via pre-flight check if running CBT namespace.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-30373: Revert UT and integration test hacks.  Need to mock the functionality correctly.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: Increase pre-flight daemonset ready wait to 10mins.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-28826: fix 'TestValidate' UT.  Don't error if pre-flight daemonset exists.

Signed-off-by: Jose Rivera <jose@portworx.com>

* Only run preflight if AWS.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-30496: If the user intended to use dmthin. The '-T dmthin' will exist in the stc before preflight is ran.  If preflight fails in this case don't start.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-30496: If preflight enables DMthin add a 64G metadata drive.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-30496: Review fixes.

Signed-off-by: Jose Rivera <jose@portworx.com>

* PWX-30496: add the metadata device in both cases where the user has passed -T dmthin or we added it for them.

Signed-off-by: Jose Rivera <jose@portworx.com>

---------

Signed-off-by: Harsh Desai <hadesai@purestorage.com>
Signed-off-by: Jose Rivera <jose@portworx.com>
Signed-off-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>
Signed-off-by: Naveen Revanna <nrevanna@purestorage.com>
Co-authored-by: Harsh Desai <hadesai@purestorage.com>
Co-authored-by: CNBU Jenkins <cnbu-jenkins@purestorage.com>
Co-authored-by: Jiafeng Liao <jliao@purestorage.com>
Co-authored-by: Piyush Nimbalkar <pnimbalkar@purestorage.com>
Co-authored-by: Naveen Revanna <83608369+nrevanna@users.noreply.github.com>
Signed-off-by: Jose Rivera <jose@portworx.com>
@@ -3758,104 +3792,6 @@ func TestDeleteStorageClusterWithFinalizers(t *testing.T) {
require.True(t, errors.IsNotFound(err))
}

func TestDeleteStorageClusterShouldSetTelemetryCertOwnerRef(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to re-add this test I believe....

Signed-off-by: Jose Rivera <jose@portworx.com>
Copy link
Member

@piyush-nimbalkar piyush-nimbalkar left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Let's just add that one missing cherrypick.

@@ -223,6 +237,17 @@ func (h *Handler) processMigration(
// events and conditions in the status. That way there is a more permanent record that
// this cluster is a result of migration. After we have added that we can remove the
// migration-approved annotation after a successful migration.

// Mark migration as completed in the cluster condition list
util.UpdateStorageClusterCondition(updatedCluster, &corev1.ClusterCondition{
Copy link
Member

Choose a reason for hiding this comment

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

There was a bug fix around this in this PR - #870. Let's cherrypick that too.

@piyush-nimbalkar piyush-nimbalkar changed the base branch from px-rel-23.4.0 to px-rel-23.5.0 April 26, 2023 22:35
@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Patch coverage: 72.86% and project coverage change: -0.47 ⚠️

Comparison is base (03df09e) 81.21% compared to head (e1fc287) 80.74%.

❗ Current head e1fc287 differs from pull request most recent head 683cc7c. Consider uploading reports for the commit 683cc7c to get more accurate results

Additional details and impacted files
@@                Coverage Diff                @@
##           px-rel-23.5.0    #1021      +/-   ##
=================================================
- Coverage          81.21%   80.74%   -0.47%     
=================================================
  Files                 52       57       +5     
  Lines              15069    15807     +738     
=================================================
+ Hits               12238    12764     +526     
- Misses              2016     2171     +155     
- Partials             815      872      +57     
Impacted Files Coverage Δ
drivers/storage/portworx/cloud_storage.go 91.37% <ø> (ø)
...e/portworx/component/securitycontextconstraints.go 82.51% <ø> (ø)
pkg/preflight/utils.go 33.33% <33.33%> (ø)
drivers/storage/portworx/status.go 79.02% <47.22%> (-11.07%) ⬇️
pkg/preflight/aws.go 62.28% <62.28%> (ø)
pkg/migration/migration.go 73.44% <63.15%> (-0.34%) ⬇️
pkg/controller/storagecluster/storagecluster.go 79.28% <68.60%> (-1.24%) ⬇️
drivers/storage/portworx/preflight.go 75.08% <75.08%> (ø)
pkg/preflight/preflight.go 78.72% <78.72%> (ø)
drivers/storage/portworx/portworx.go 89.29% <86.82%> (-1.42%) ⬇️
... and 8 more

... and 1 file 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.

@jrivera-px jrivera-px merged commit b9d3ddb into px-rel-23.5.0 Apr 27, 2023
@piyush-nimbalkar piyush-nimbalkar deleted the pre-check-px-rel-23.4.0 branch September 15, 2023 17:44
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.

None yet

5 participants