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

Completed Source_PhysicalServer workflow #4

Open
wants to merge 2 commits into
base: v1.1
Choose a base branch
from

Conversation

ggCohesity
Copy link

No description provided.

Copy link
Contributor

@pyashish pyashish left a comment

Choose a reason for hiding this comment

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

Minor nits, Go ahead, fix and merge

Type: schema.TypeBool,
Optional: true,
Default: true,
Description: `Forcefully register physical server to target cluster`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Double quotes .. consistent with below.

}

func resourceCohesitySourcePhysicalServerCreate(resourceData *schema.ResourceData, configMetaData interface{}) error {
log.Printf("[INFO] Starting PS Registration")
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace PS with Physical Server.


if err != nil {
log.Printf(err.Error())
return errors.New("Failed to register Cohesity protection source")
Copy link
Contributor

Choose a reason for hiding this comment

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

better to add the protection source type ? Physical Server

}

func resourceCohesitySourcePhysicalServerRead(resourceData *schema.ResourceData, configMetaData interface{}) error {
log.Printf("Starting Read")
Copy link
Contributor

Choose a reason for hiding this comment

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

Be consistent with logging [INFO] or [DEBUG]

Copy link
Contributor

@anvesh-cohesity anvesh-cohesity left a comment

Choose a reason for hiding this comment

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

can you change the file name to use snake cases (change this physicalServer to physical_server). Didn't find the resource inclusion in the provider.go file, without this the resource wouldn't be included in the provider

cohesity/resource_cohesity_source_physicalServer.go Outdated Show resolved Hide resolved
return errors.New("Failed to authenticate with Cohesity")
}

protectionSourceID, _ := strconv.ParseInt(resourceData.Id(), 10, 64)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a comment here to tell what strconv.ParseInt(resourceData.Id(), 10, 64) this is doing

if *protectionSource.ProtectionSource.Name == endpoint {
log.Printf("[INFO] Found the Physical Server protection source %s on cohesity cluster", endpoint)
//validate the Physical Server Source Registration
if *protectionSource.LogicalSize == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you checking the logical size here, we can just check the name, if its there then return nil


// *protectionSource.RegistrationInfo.AccessInfo.Endpoint != endpoint {
// return fmt.Errorf("Failed to valaidate created physical server source")
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you pls remove all the commented code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants