Skip to content

Conversation

@thunderboltsid
Copy link
Contributor

@thunderboltsid thunderboltsid commented Jan 31, 2022

Add a nutanix subcommand to ccoctl for handling nutanix CredentialsRequest objects.

x-ref: https://issues.redhat.com/browse/CCO-108

@openshift-ci openshift-ci bot requested review from joelddiaz and suhanime January 31, 2022 12:51
@codecov
Copy link

codecov bot commented Jan 31, 2022

Codecov Report

Merging #450 (bdd8533) into master (aa55102) will decrease coverage by 0.42%.
The diff coverage is 53.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #450      +/-   ##
==========================================
- Coverage   45.92%   45.50%   -0.43%     
==========================================
  Files          92       94       +2     
  Lines        9300     9389      +89     
==========================================
+ Hits         4271     4272       +1     
- Misses       4520     4608      +88     
  Partials      509      509              
Impacted Files Coverage Δ
cmd/ccoctl/main.go 0.00% <0.00%> (ø)
.../cmd/provisioning/nutanix/create_shared_secrets.go 0.00% <0.00%> (ø)
pkg/cmd/provisioning/nutanix/nutanix.go 0.00% <0.00%> (ø)
pkg/aws/mock/client_generated.go 56.59% <40.17%> (ø)
pkg/alibabacloud/mock/client_generated.go 54.00% <52.85%> (ø)
pkg/gcp/mock/client_generated.go 87.72% <84.61%> (ø)
pkg/ibmcloud/mock/client_generated.go 82.35% <85.29%> (ø)
pkg/apis/cloudcredential/v1/register.go 88.23% <100.00%> (+0.73%) ⬆️
pkg/azure/mock/client_generated.go 100.00% <100.00%> (ø)
... and 4 more

Copy link
Contributor

@joelddiaz joelddiaz left a comment

Choose a reason for hiding this comment

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

Also, can you split up the vendoring into its own separate commit as part of this single PR?

The updated openshift/api has support for Nutanix plattform.
@thunderboltsid
Copy link
Contributor Author

Also, can you split up the vendoring into its own separate commit as part of this single PR?

Sure

Copy link
Contributor

@akhil-rane akhil-rane left a comment

Choose a reason for hiding this comment

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

We need to add code to delete credentials. It can be added in a separate PR.

@joelddiaz
Copy link
Contributor

We need to add code to delete credentials. It can be added in a separate PR.

I think since Nutanix is effectively implementing Passthrough mode with the same username/password for all components, there is really nothing to delete, since the only thing created were the Secret yaml files that are used to be fed to the installer.

@akhil-rane
Copy link
Contributor

We need to add code to delete credentials. It can be added in a separate PR.

I think since Nutanix is effectively implementing Passthrough mode with the same username/password for all components, there is really nothing to delete, since the only thing created were the Secret yaml files that are used to be fed to the installer.

yup my bad, No delete code is needed

@thunderboltsid thunderboltsid changed the title Add nutanix credential handling with manual mode [WIP] Add nutanix credential handling with manual mode Feb 9, 2022
@thunderboltsid thunderboltsid changed the title [WIP] Add nutanix credential handling with manual mode Add nutanix credential handling with manual mode Feb 9, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 9, 2022
@thunderboltsid thunderboltsid force-pushed the nutanix-plaform-manual branch 3 times, most recently from 6d2a355 to 729e8ce Compare February 14, 2022 14:13
@thunderboltsid
Copy link
Contributor Author

/retest

1 similar comment
@thunderboltsid
Copy link
Contributor Author

/retest

@thunderboltsid
Copy link
Contributor Author

/remove-label do-not-merge/work-in-progress

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 15, 2022

@thunderboltsid: The label(s) /remove-label do-not-merge/work-in-progress cannot be applied. These labels are supported: platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, downstream-change-needed, backport-risk-assessed, cherry-pick-approved

Details

In response to this:

/remove-label do-not-merge/work-in-progress

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.

@thunderboltsid thunderboltsid changed the title Add nutanix credential handling with manual mode Add nutanix credentials handling with manual mode Feb 15, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 15, 2022
@thunderboltsid
Copy link
Contributor Author

/retest

1 similar comment
@thunderboltsid
Copy link
Contributor Author

/retest

@jianping-shu
Copy link

@thunderboltsid It worked but the weird thing is two same credentials-request were extracted, bug?
After running ccoctl, only one credentials file was generated.
[cloud-user@preserve-for-hive-test ]$ ./oc -a ~/.pull-secret adm release extract --credentials-requests --cloud=nutanix --to=./credentials_requests_new public.ecr.aws/g7v6l0u0/openshift/origin-release:nightly
[cloud-user@preserve-for-hive-test ]$ ls -l ./credentials_requests_new
total 8
-rw-rw-r--. 1 cloud-user cloud-user 482 Mar 9 20:49 0000_30_machine-api-operator_00_credentials-request.yaml
-rw-rw-r--. 1 cloud-user cloud-user 482 Mar 9 20:49 0000_30_machine-api-operator_00_credentials-request_2.yaml
[cloud-user@preserve-for-hive-test ]$ ./ccoctl nutanix create-shared-secrets --credentials-requests-dir=./credentials_requests_new --output-dir=./nutanix_secrets_new
2022/03/09 22:13:34 Saved credentials configuration to: nutanix_secrets_new/manifests/openshift-machine-api-nutanix-credentials-credentials.yaml
2022/03/09 22:13:34 Saved credentials configuration to: nutanix_secrets_new/manifests/openshift-machine-api-nutanix-credentials-credentials.yaml

@jianping-shu
Copy link

@thunderboltsid @akhil-rane I've updated case OCP-48967 for --enable-tech-preview, PTAL and let me know if any comment
One more issue is that the ccoctl generated nutanix-credentials secret yaml has format issue(See below error) and it doesn't have "type: Opaque" like others.
jianpingshu@jshu-mac /tmp % oc apply -f openshift-machine-api-nutanix-credentials-credentials.yaml
error: error parsing openshift-machine-api-nutanix-credentials-credentials.yaml: error converting YAML to JSON: yaml: line 2: mapping values are not allowed in this context

@thunderboltsid
Copy link
Contributor Author

@thunderboltsid It worked but the weird thing is two same credentials-request were extracted, bug?
After running ccoctl, only one credentials file was generated.
[cloud-user@preserve-for-hive-test ]$ ./oc -a ~/.pull-secret adm release extract --credentials-requests --cloud=nutanix --to=./credentials_requests_new public.ecr.aws/g7v6l0u0/openshift/origin-release:nightly
[cloud-user@preserve-for-hive-test ]$ ls -l ./credentials_requests_new
total 8
-rw-rw-r--. 1 cloud-user cloud-user 482 Mar 9 20:49 0000_30_machine-api-operator_00_credentials-request.yaml
-rw-rw-r--. 1 cloud-user cloud-user 482 Mar 9 20:49 0000_30_machine-api-operator_00_credentials-request_2.yaml
[cloud-user@preserve-for-hive-test ]$ ./ccoctl nutanix create-shared-secrets --credentials-requests-dir=./credentials_requests_new --output-dir=./nutanix_secrets_new
2022/03/09 22:13:34 Saved credentials configuration to: nutanix_secrets_new/manifests/openshift-machine-api-nutanix-credentials-credentials.yaml
2022/03/09 22:13:34 Saved credentials configuration to: nutanix_secrets_new/manifests/openshift-machine-api-nutanix-credentials-credentials.yaml

Indeed, I can see there are two credentials requests being generated. If there is a bug, it's in the oc tool but most likely the release image had two copies of the same credentials requests (harmless imo).

Add docs on using ccoctl for Nutanix.
@thunderboltsid thunderboltsid force-pushed the nutanix-plaform-manual branch from e26aa22 to e5bc157 Compare March 14, 2022 13:54
@thunderboltsid
Copy link
Contributor Author

@thunderboltsid @akhil-rane I've updated case OCP-48967 for --enable-tech-preview, PTAL and let me know if any comment
One more issue is that the ccoctl generated nutanix-credentials secret yaml has format issue(See below error) and it doesn't have "type: Opaque" like others.
jianpingshu@jshu-mac /tmp % oc apply -f openshift-machine-api-nutanix-credentials-credentials.yaml
error: error parsing openshift-machine-api-nutanix-credentials-credentials.yaml: error converting YAML to JSON: yaml: line 2: mapping values are not allowed in this context

@jianping-shu that's a good catch. Seems like this was due to a whitespace issue. Addressed this now.

@jianping-shu
Copy link

@thunderboltsid Thanks for update!
As to duplicate credentials-request files issue, pls. raise it to the appropriate PR/ticket. It might be harmless but confusing as well.

I rebuilt with the updated PR and tried ccoctl again. Seems the generated secret yaml file is still in wrong format. Pls. help check it.
To diff the new secret file with previous one,
[cloud-user@preserve-for-hive-test manifests]$ diff openshift-machine-api-nutanix-credentials-credentials.yaml ../../nutanix_secrets_preview/manifests/openshift-machine-api-nutanix-credentials-credentials.yaml
2,3c2,3
< kind: Secret
< metadata:

kind: Secret
metadata:
6,7c6
< type: Opaque
< data:


data:

[cloud-user@preserve-for-hive-test manifests]$ oc apply -f openshift-machine-api-nutanix-credentials-credentials.yaml
Error from server (BadRequest): error when creating "openshift-machine-api-nutanix-credentials-credentials.yaml": Secret in version "v1" cannot be handled as a Secret: illegal base64 data at input byte 5

@thunderboltsid thunderboltsid force-pushed the nutanix-plaform-manual branch from e5bc157 to 3e4de00 Compare March 15, 2022 09:50
@thunderboltsid
Copy link
Contributor Author

/retest-required

@thunderboltsid
Copy link
Contributor Author

thunderboltsid commented Mar 15, 2022

@thunderboltsid Thanks for update! As to duplicate credentials-request files issue, pls. raise it to the appropriate PR/ticket. It might be harmless but confusing as well.

I rebuilt with the updated PR and tried ccoctl again. Seems the generated secret yaml file is still in wrong format. Pls. help check it.

To diff the new secret file with previous one,
[cloud-user@preserve-for-hive-test manifests]$ diff openshift-machine-api-nutanix-credentials-credentials.yaml ../../nutanix_secrets_preview/manifests/openshift-machine-api-nutanix-credentials-credentials.yaml
2,3c2,3
< kind: Secret
< metadata:

kind: Secret
metadata:
6,7c6
< type: Opaque
< data:

data:

[cloud-user@preserve-for-hive-test manifests]$ oc apply -f openshift-machine-api-nutanix-credentials-credentials.yaml Error from server (BadRequest): error when creating "openshift-machine-api-nutanix-credentials-credentials.yaml": Secret in version "v1" cannot be handled as a Secret: illegal base64 data at input byte 5

@jianping-shu Looks like something to do with the encoding for base64. Can you try it out again?

@jianping-shu
Copy link

@thunderboltsid Weird! Same error again. Did you encounter this in your env?
[cloud-user@preserve-for-hive-test ]$ diff nutanix_secrets_latest_new/manifests/openshift-machine-api-nutanix-credentials-credentials.yaml nutanix_secrets_latest/manifests/openshift-machine-api-nutanix-credentials-credentials.yaml
11c11
< NUTANIX_PASSWORD: aaaaaa
\ No newline at end of file

NUTANIX_PASSWORD: aaaaaa

[cloud-user@preserve-for-hive-test ]$ oc apply -f nutanix_secrets_latest_new/manifests/openshift-machine-api-nutanix-credentials-credentials.yaml
Error from server (BadRequest): error when creating "nutanix_secrets_latest2/manifests/openshift-machine-api-nutanix-credentials-credentials.yaml": Secret in version "v1" cannot be handled as a Secret: illegal base64 data at input byte 5

[cloud-user@preserve-for-hive-test ]$ oc version
Client Version: 4.10.0-0.nightly-2021-12-06-201335
Server Version: 4.10.0-0.nightly-2022-03-14-215709
Kubernetes Version: v1.23.3+e419edf

Add a nutanix subcommand to ccoctl for handling nutanix CredentialsRequest objects.
@thunderboltsid thunderboltsid force-pushed the nutanix-plaform-manual branch from 3e4de00 to bdd8533 Compare March 15, 2022 12:05
@jianping-shu
Copy link

@thunderboltsid The test case passed now after getting the commit pushed correctly. Thanks for the support!
BTW: Don't forget the duplicate credentials-request files issue.
[cloud-user@preserve-for-hive-test $ oc apply -f nutanix_secrets_latest_new/manifests/openshift-machine-api-nutanix-credentials-credentials.yaml
secret/nutanix-credentials created

@jianping-shu
Copy link

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Mar 15, 2022
@thunderboltsid
Copy link
Contributor Author

/retest-required

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 15, 2022

@thunderboltsid: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

@akhil-rane
Copy link
Contributor

/lgtm

@akhil-rane
Copy link
Contributor

/assign @jeana-redhat @sferich888

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 16, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 16, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akhil-rane, thunderboltsid

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@akhil-rane
Copy link
Contributor

/label docs-approved

Docs looks good as confirmed by @jeana-redhat

@akhil-rane
Copy link
Contributor

/label docs-approved

@sferich888
Copy link

/label px-approved

@openshift-ci openshift-ci bot added docs-approved Signifies that Docs has signed off on this PR px-approved Signifies that Product Support has signed off on this PR labels Mar 16, 2022
@openshift-merge-robot openshift-merge-robot merged commit ed06129 into openshift:master Mar 16, 2022
ming1013 pushed a commit to ming1013/cloud-credential-operator that referenced this pull request Dec 15, 2025
…aform-manual

Add nutanix credentials handling with manual mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants