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

Granular CRN for image datasources and resources #5646

Merged
merged 1 commit into from
Oct 11, 2024
Merged

Conversation

Alexander-Kita
Copy link
Collaborator

@Alexander-Kita Alexander-Kita commented Sep 17, 2024

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Output from acceptance testing:

pi_catalog_images (d):

=== RUN   TestAccIBMPICatalogImagesDataSourceBasic
--- PASS: TestAccIBMPICatalogImagesDataSourceBasic (52.36s)
PASS
=== RUN   TestAccIBMPICatalogImagesDataSourceVTL
--- PASS: TestAccIBMPICatalogImagesDataSourceVTL (51.41s)
PASS
=== RUN   TestAccIBMPICatalogImagesDataSourceSAP
--- PASS: TestAccIBMPICatalogImagesDataSourceSAP (74.17s)
PASS
=== RUN   TestAccIBMPICatalogImagesDataSourceSAPAndVTL
--- PASS: TestAccIBMPICatalogImagesDataSourceSAPAndVTL (61.02s)
PASS

pi_image (d):

=== RUN   TestAccIBMPIImageDataSource_basic
--- PASS: TestAccIBMPIImageDataSource_basic (17.08s)
PASS

pi_images (d):

=== RUN   TestAccIBMPIImagesDataSource_basic
--- PASS: TestAccIBMPIImagesDataSource_basic (28.87s)
PASS

pi_image (r):

=== RUN   TestAccIBMPIImagebasic
--- PASS: TestAccIBMPIImagebasic (40.15s)
PASS

(Used private bucket)

=== RUN   TestAccIBMPIImageCOSPublicImport
--- PASS: TestAccIBMPIImageCOSPublicImport (1227.62s)
PASS
=== RUN   TestAccIBMPIImageBYOLImport
--- PASS: TestAccIBMPIImageBYOLImport (1817.78s)
PASS
--- PASS: TestAccIBMPIImageUserTags (38.51s)
PASS

@Alexander-Kita Alexander-Kita added the service/Power Systems Issues related to Power Systems label Sep 17, 2024
@Alexander-Kita
Copy link
Collaborator Author

@yussufsh, @hkantare Can you please review this PR which part of power Q3

ismirlia
ismirlia previously approved these changes Sep 18, 2024
ismirlia
ismirlia previously approved these changes Sep 20, 2024
ismirlia
ismirlia previously approved these changes Sep 23, 2024
Arg_UserTags: {
Description: "The user tags attached to this resource.",
Elem: &schema.Schema{Type: schema.TypeString},
ForceNew: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

change in tags shouldn't be force_new. Either suppress the diff or provide update support

Copy link
Collaborator

@ismirlia ismirlia Sep 24, 2024

Choose a reason for hiding this comment

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

User tags cannot be updated. Why is this incorrect? We don't want users to try to update the user tag field thinking it will do something.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hkantare Since are only supporting this at creation, forceNew is okay here. The management of the userTags will be handle by Global Tagging at this moment. Like @ismirlia said we won't be allowing the user to update this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@michaelkad Why can't we adapt the same approach like other services team
During create/update always use Global Tagging API's.
This gives capability for user's to add /update tags

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hkantare We will make a change to let the user update the tags using the Global Tagging API's.

Elem: &schema.Schema{Type: schema.TypeString},
ForceNew: true,
Optional: true,
Type: schema.TypeList,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change the Type to schema.TypeSet to handle the unordered collection. List is ordered collection even if their change in the position of tags it will show diff

"tags": {
				Type:        schema.TypeSet,
				Optional:    true,
				Elem:        &schema.Schema{Type: schema.TypeString},
				Set:         schema.HashString,
				Description: "Tags set for the resource",
			},
			```

ibm/service/power/resource_ibm_pi_image.go Show resolved Hide resolved
michaelkad
michaelkad previously approved these changes Sep 26, 2024
@michaelkad
Copy link
Collaborator

@hkantare we have updated the pr

ismirlia
ismirlia previously approved these changes Sep 26, 2024
ismirlia
ismirlia previously approved these changes Sep 27, 2024
michaelkad
michaelkad previously approved these changes Oct 1, 2024
michaelkad
michaelkad previously approved these changes Oct 2, 2024
@kavya498 kavya498 merged commit fef11ba into master Oct 11, 2024
1 check passed
@Alexander-Kita Alexander-Kita deleted the crn-images branch November 19, 2024 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service/Power Systems Issues related to Power Systems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants