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

[Feat.] ECS: Add metadata with agency_name to ecs_instance_v1 #2835

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

Epimetheus89
Copy link
Contributor

@Epimetheus89 Epimetheus89 commented Feb 21, 2025

Summary of the Pull Request

Add metadata field to ecs_instance_v1 resource. With metadata it is possible to set the agency_name used by ICAgent.

PR Checklist

Acceptance Steps Performed


@Epimetheus89 Epimetheus89 marked this pull request as ready for review February 21, 2025 09:06
Copy link
Member

@muneeb-jan muneeb-jan left a comment

Choose a reason for hiding this comment

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

Also, add acceptance tests for changes made in the code.
Refer to existing tests in the directory opentelekomcloud/acceptance/ecs
Paste it in the description. It should look something like...

=== RUN   TestAccSomething_basic
--- PASS: TestAccSomething_basic (290.91s)
PASS

You can use the run and debug feature in vscode for this.
Click on settings icon on top left which creates launch.json. In launch.json you can add following:

{
    "version": "0.2.0",
    "configurations": [
        {
            "name": "ECS instance V1 test",
            "type": "go",
            "request": "launch",
            "mode": "test",
            "program": "${workspaceFolder}/opentelekomcloud/acceptance/ecs",
            "envFile": "${workspaceFolder}/test.env",
            "args": ["-test.v", "-test.run", "^TestAccSomething_basic$"]
        },
    ]
}

Here test.env is your environment file which can contain necessary flags such as

OS_CLOUD=your_cloud_name
OS_ACCESS_KEY=xxx
OS_SECRET_KEY=yyy
OS_PROJECT_ID=xxx
OS_SUBNET_ID=yyy
OS_VPC_ID=xxx
OS_NETWORK_ID=yyy
# so on and so forth.

and TestAccSomething_basic is replaced by the name of your test. (recommended to just add metadata field to ECS basic test and run that)

Finally, add a release note. we use reno to add releasenotes. Something like

reno new ecs-instance-add-metadata-field

refer to other release notes.

@@ -394,6 +402,12 @@ func resourceEcsInstanceV1Read(ctx context.Context, d *schema.ResourceData, meta
d.Set("volumes_attached", volumeList),
)

mErr = multierror.Append(mErr,
d.Set("metadata", map[string]string{
"agency_name": server.Metadata.AgencyName,
Copy link
Member

Choose a reason for hiding this comment

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

"agency_name" needs to be added to schema as a subfield of metadata (see comment for metadata) and then set properly if the value is returned from the cloud.

Refer to how compound fields such as nics, etc are handled in the code.

@@ -124,6 +124,13 @@ func ResourceEcsInstanceV1() *schema.Resource {
},
},
},
"metadata": {
Copy link
Member

@muneeb-jan muneeb-jan Feb 24, 2025

Choose a reason for hiding this comment

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

Since metadata has predefined subfields. It is recommended to use something like

"metadata": {
        Type:     schema.TypeList,
        Required: true,
        MaxItems: 1,
        Elem: &schema.Resource{
	        Schema: map[string]*schema.Schema{
		        "agency_name": {
			        Type:     schema.TypeString,
			        Optional: true,
		        },
	        },
        },
},

@@ -312,6 +314,10 @@ The `nics` block supports:
* `ip_address` - (Optional, String, ForceNew) Specifies a fixed IPv4 address to be used on this
network. Changing this creates a new server.

The `metadata` block supports:

* `agency_name` - (Optional) Association to an [agency](identity_agency_v3.md)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* `agency_name` - (Optional) Association to an [agency](identity_agency_v3.md)
* `agency_name` - (Optional, String) Association to an [agency](identity_agency_v3.md)

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.

2 participants