-
Notifications
You must be signed in to change notification settings - Fork 102
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
Aci resource migration esg #1223
Aci resource migration esg #1223
Conversation
db6f3ce
to
7bc427f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just have question about that json file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this file for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file contains an overview of the provider schema retrieved with: terraform providers schema -json | jq > schema-doc-new.json. This is used read old the old attribute types/name, which need to be migrated. This way we do not have to manually input all this information into definition files.
I have added the commit tag in case we ever want to look back, or generate a new file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that generation part of the go generate process?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No this is a static file, if we would regenerate on every provider update the problem would be that the old schema used for migration is lost and thus the mapping would not work. So the assumption is a snapshot in time in which we do not make changes anymore to schema of existing resources, where if something needs to be adjusted we would migrate it to framework.
ce55265
to
baeb7b5
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1223 +/- ##
==========================================
- Coverage 87.09% 85.08% -2.02%
==========================================
Files 45 59 +14
Lines 11802 16619 +4817
==========================================
+ Hits 10279 14140 +3861
- Misses 1035 1802 +767
- Partials 488 677 +189 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
gen/definitions/classes.yaml
Outdated
@@ -20,6 +23,7 @@ vzCPIf: | |||
rtctrlProfile: | |||
resource_name: "route_control_profile" | |||
rn_format: "/prof-{name}" | |||
exclude: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is exclude used here? Did we introduce the ability to not generate resources if exclude is set to true in the generator and its meta is included?
{ | ||
annotation = "annotation_1" | ||
priority = "level1" | ||
contract_interface_name = aci_contract_interface.example.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have a resource aci_contract_interface? Is this the right value?
0fbda53
…es in class definition file
7589916
to
7d7be7a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gen/definitions/classes.yaml
Outdated
- "Tenants -> Networkings -> L3Outs -> External EPGs -> Contracts" | ||
- "Tenants -> Networkings -> L2Outs -> External EPGs -> Contracts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- "Tenants -> Networkings -> L3Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networkings -> L2Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networking -> L3Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networking -> L2Outs -> External EPGs -> Contracts" |
gen/definitions/classes.yaml
Outdated
- "Tenants -> Networkings -> L3Outs -> External EPGs -> Contracts" | ||
- "Tenants -> Networkings -> L2Outs -> External EPGs -> Contracts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- "Tenants -> Networkings -> L3Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networkings -> L2Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networking -> L3Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networking -> L2Outs -> External EPGs -> Contracts" |
gen/definitions/classes.yaml
Outdated
- "Tenants -> Networkings -> L3Outs -> External EPGs -> Contracts" | ||
- "Tenants -> Networkings -> L2Outs -> External EPGs -> Contracts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- "Tenants -> Networkings -> L3Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networkings -> L2Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networking -> L3Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networking -> L2Outs -> External EPGs -> Contracts" |
gen/definitions/classes.yaml
Outdated
- "Tenants -> Networkings -> L3Outs -> External EPGs -> Contracts" | ||
- "Tenants -> Networkings -> L2Outs -> External EPGs -> Contracts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- "Tenants -> Networkings -> L3Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networkings -> L2Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networking -> L3Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networking -> L2Outs -> External EPGs -> Contracts" |
gen/definitions/classes.yaml
Outdated
ui_locations: | ||
- "Tenants -> Application Profiles -> Application EPGs -> Contracts" | ||
- "Tenants -> Application Profiles -> Endpoint Security Groups -> Contracts" | ||
- "Tenants -> Networkings -> L3Outs -> External EPGs -> Contracts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- "Tenants -> Networkings -> L3Outs -> External EPGs -> Contracts" | |
- "Tenants -> Networking -> L3Outs -> External EPGs -> Contracts" |
gen/definitions/classes.yaml
Outdated
- "Tenants -> Networkings -> L3Outs -> External EPGs" | ||
- "Tenants -> Networkings -> L2Outs -> External EPGs" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- "Tenants -> Networkings -> L3Outs -> External EPGs" | |
- "Tenants -> Networkings -> L2Outs -> External EPGs" | |
- "Tenants -> Networking -> L3Outs -> External EPGs" | |
- "Tenants -> Networking -> L2Outs -> External EPGs" |
gen/definitions/properties.yaml
Outdated
@@ -12,14 +12,17 @@ global: | |||
id: "The identifier of the %s object." | |||
tnVzOOBBrCPName: "The name of the Out Of Band Contract object." | |||
tnRtctrlProfileName: "The name of the Route Control Profile object." | |||
prio: "The QoS priority class identifierq of the %s object." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prio: "The QoS priority class identifierq of the %s object." | |
prio: "The QoS priority class identifier of the %s object." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
fixes #1157