MGMT-22189: Order dual-stack networks when fetching from DB #8488
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci-robot
added
the
jira/valid-reference
|
@CrystalChun: This pull request references MGMT-22189 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
WalkthroughRemoved automatic in-code reordering of cluster network records; introduced explicit primary-IP-stack determination and validation; moved network-family ordering into DB load callbacks; added documentation for primary-stack behavior and tests covering single-/dual-stack ordering and retrieval. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to ⛔ Files ignored due to path filters (258)
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
Comment |
openshift-ci
bot
added
the
size/L
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CrystalChun The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
CrystalChun
force-pushed
the
dual-stack-ip-order
branch
12 times, most recently
from
900be67 to
4f0addb
Compare
CrystalChun
commented
Dec 8, 2025
internal/bminventory/inventory.go
Outdated
| if params.NewClusterParams.ClusterNetworks == nil { | ||
| params.NewClusterParams.ClusterNetworks = []*models.ClusterNetwork{ | ||
| {Cidr: models.Subnet(b.Config.DefaultClusterNetworkCidr), HostPrefix: b.Config.DefaultClusterNetworkHostPrefix}, | ||
| {Cidr: models.Subnet(b.Config.DefaultClusterNetworkCidr), HostPrefix: b.Config.DefaultClusterNetworkHostPrefix}, //TODO: what if it's a dual-stack cluster with primary IPv6 |
Contributor
Author
There was a problem hiding this comment.
@linoyaslan Would appreciate your thoughts!
Contributor
There was a problem hiding this comment.
yeah, I’m aware of this. it was already discussed during the initial implementation of the feature. In the UI, this is handled by adjusting the defaults for the user. However, on the API side, the user still needs to handle this manually. If they don’t, they’ll receive an error about an inconsistent family stack, and the error message is informative enough to guide them on what needs to be changed.
That said, due to some recent changes we made after the initial implementation, I think we now exec the defaults method after the initial primary IP stack computation. This might allow us to introduce an additional defaulting function for primary IPv6?
Contributor
Author
There was a problem hiding this comment.
Ah gotcha, thank you for the explanation!
I think we now exec the defaults method after the initial primary IP stack computation. This might allow us to introduce an additional defaulting function for primary IPv6?
Yes, we do! Perhaps we can add the defaulting in a different issue/PR then as a follow up. I've edited the todo for the function.
CrystalChun
force-pushed
the
dual-stack-ip-order
branch
from
4f0addb to
78670aa
Compare
openshift-merge-robot
added
the
needs-rebase
CrystalChun
force-pushed
the
dual-stack-ip-order
branch
from
78670aa to
3d8893e
Compare
openshift-merge-robot
removed
the
needs-rebase
linoyaslan
reviewed
Dec 9, 2025
Contributor
|
looks good to me :) I’d like to get @danmanor’s opinion, since he was involved in my implementation and may have additional insights here |
CrystalChun
force-pushed
the
dual-stack-ip-order
branch
2 times, most recently
from
9e59ae9 to
206cd9d
Compare
openshift-merge-robot
removed
the
needs-rebase
CrystalChun
force-pushed
the
dual-stack-ip-order
branch
from
6198321 to
8adf84a
Compare
linoyaslan
reviewed
Dec 19, 2025
| // Verify the column type after migration | ||
| colType, err = getColumnType(dbName, &common.Cluster{}, "primary_ip_stack") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(colType).To(Equal("int4")) |
Contributor
There was a problem hiding this comment.
I’m not completely sure when the column type is shown as int4 and when as integer. In the migration you check for integer / bigint, but here the type appears as int4.
this might depend on what you specify in the SQL vs how the database reports it? anyway, I just wanted to point this out so we can double-check that this is expected and that we are not missing anything here.
https://www.postgresql.org/docs/current/datatype-numeric.html#DATATYPE-INT
"SQL only specifies the integer types integer (or int), smallint, and bigint. The type names int2, int4, and int8 are extensions, which are also used by some other SQL database systems."
Contributor
Author
There was a problem hiding this comment.
The getColumnType function uses GORM's DatabaseTypeName() which returns PostgreSQL's internal type name:
INTEGER → returns int4
BIGINT → returns int8
TEXT → returns text
Thanks for bringing this up! It is interesting that there's a discrepancy
linoyaslan
reviewed
Dec 19, 2025
| var invalidValues []string | ||
| err = tx.Raw(fmt.Sprintf(` | ||
| SELECT DISTINCT %s FROM %s | ||
| WHERE %s !~ '^[0-9a-fA-F.:]+(/[0-9]+)?$' |
Contributor
There was a problem hiding this comment.
doesn’t this regex also allow something like 999.888.777.666? but the db might break in the migration since 999 isn’t a valid octet which inet/cidr expect? not 100% sure about that, but probably good to check
Contributor
There was a problem hiding this comment.
oh I now see your comment "This regex is permissive - it catches obvious garbage but lets PostgreSQL do final validation". when does Postgres validate it? During cleanAndValidateNetworkData, or only later?
Contributor
Author
There was a problem hiding this comment.
That's a good catch! The validation for converting happens just a bit later down the codeblock
Let me add a warning for users there too
CrystalChun
force-pushed
the
dual-stack-ip-order
branch
from
9b1b7c6 to
9cce726
Compare
Change primary ip stack to an int to simplify the SQL query. Includes migrations to move any previously filled column where primary ip stack was a text type.
Previously, dual-stack network ordering was not guaranteed when fetching a cluster from the DB since the fields are text fields. In order to guarantee the order, every fetch for the cluster from the db will add a postgres clause for the network subtables. The postgres clause gets ip family of the network ip address or cidrs, and will then order them based on the primary_ip_family column for that cluster.
Postgres types inet and cidr have additional stricter validation requirements. Changes made: - cidrs cannot have bits to the right of the mask specified so the cidrs in the tests are edited to respect that - inet and cidrs are not allowed to be empty or null so any test that previously specified an empty or null ip or cidr are removed. - Default cidrs in the inventory are configured in tests ahead of time so they won't be empty during the tests
CrystalChun
force-pushed
the
dual-stack-ip-order
branch
5 times, most recently
from
5c8d5b9 to
8e35ea6
Compare
CrystalChun
force-pushed
the
dual-stack-ip-order
branch
from
8e35ea6 to
994cad4
Compare
|
@CrystalChun: This pull request references MGMT-22189 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
CrystalChun
force-pushed
the
dual-stack-ip-order
branch
2 times, most recently
from
e2839f9 to
cd089fc
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (183)
api/go.sumis excluded by!**/*.sumapi/vendor/github.com/jackc/pgio/.travis.ymlis excluded by!**/vendor/**api/vendor/github.com/jackc/pgio/LICENSEis excluded by!**/vendor/**api/vendor/github.com/jackc/pgio/README.mdis excluded by!**/vendor/**api/vendor/github.com/jackc/pgio/doc.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgio/write.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/CHANGELOG.mdis excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/LICENSEis excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/README.mdis excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/aclitem.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/aclitem_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/array_type.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/bit.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/bool.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/bool_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/box.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/bpchar.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/bpchar_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/bytea.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/bytea_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/cid.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/cidr.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/cidr_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/circle.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/composite_fields.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/composite_type.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/convert.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/database_sql.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/date.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/date_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/daterange.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/enum_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/enum_type.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/float4.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/float4_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/float8.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/float8_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/generic_binary.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/generic_text.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/hstore.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/hstore_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/inet.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/inet_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/int2.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/int2_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/int4.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/int4_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/int4_multirange.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/int4range.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/int8.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/int8_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/int8_multirange.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/int8range.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/interval.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/json.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/json_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/jsonb.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/jsonb_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/line.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/lseg.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/ltree.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/macaddr.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/macaddr_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/multirange.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/name.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/num_multirange.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/numeric.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/numeric_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/numrange.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/oid.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/oid_value.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/path.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/pgtype.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/pguint32.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/point.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/polygon.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/qchar.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/range.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/record.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/record_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/text.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/text_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/tid.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/time.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/timestamp.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/timestamp_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/timestamptz.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/timestamptz_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/tsrange.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/tsrange_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/tstzrange.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/tstzrange_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/typed_array.go.erbis excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/typed_array_gen.shis excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/typed_multirange.go.erbis excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/typed_multirange_gen.shis excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/typed_range.go.erbis excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/typed_range_gen.shis excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/unknown.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/uuid.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/uuid_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/varbit.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/varchar.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/varchar_array.gois excluded by!**/vendor/**api/vendor/github.com/jackc/pgtype/xid.gois excluded by!**/vendor/**api/vendor/github.com/openshift/assisted-service/models/api_vip.gois excluded by!**/vendor/**api/vendor/github.com/openshift/assisted-service/models/cluster_network.gois excluded by!**/vendor/**api/vendor/github.com/openshift/assisted-service/models/custom.gois excluded by!**/vendor/**api/vendor/github.com/openshift/assisted-service/models/ingress_vip.gois excluded by!**/vendor/**api/vendor/github.com/openshift/assisted-service/models/machine_network.gois excluded by!**/vendor/**api/vendor/github.com/openshift/assisted-service/models/service_network.gois excluded by!**/vendor/**api/vendor/github.com/openshift/assisted-service/models/verified_vip.gois excluded by!**/vendor/**api/vendor/github.com/openshift/assisted-service/models/verify_vip.gois excluded by!**/vendor/**api/vendor/modules.txtis excluded by!**/vendor/**client/go.sumis excluded by!**/*.sumclient/vendor/github.com/jackc/pgio/.travis.ymlis excluded by!**/vendor/**client/vendor/github.com/jackc/pgio/LICENSEis excluded by!**/vendor/**client/vendor/github.com/jackc/pgio/README.mdis excluded by!**/vendor/**client/vendor/github.com/jackc/pgio/doc.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgio/write.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/CHANGELOG.mdis excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/LICENSEis excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/README.mdis excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/aclitem.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/aclitem_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/array_type.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/bit.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/bool.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/bool_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/box.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/bpchar.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/bpchar_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/bytea.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/bytea_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/cid.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/cidr.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/cidr_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/circle.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/composite_fields.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/composite_type.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/convert.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/database_sql.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/date.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/date_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/daterange.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/enum_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/enum_type.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/float4.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/float4_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/float8.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/float8_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/generic_binary.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/generic_text.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/hstore.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/hstore_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/inet.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/inet_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/int2.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/int2_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/int4.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/int4_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/int4_multirange.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/int4range.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/int8.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/int8_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/int8_multirange.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/int8range.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/interval.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/json.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/json_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/jsonb.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/jsonb_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/line.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/lseg.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/ltree.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/macaddr.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/macaddr_array.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/multirange.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/name.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/num_multirange.gois excluded by!**/vendor/**client/vendor/github.com/jackc/pgtype/numeric.gois excluded by!**/vendor/**
📒 Files selected for processing (1)
api/go.mod(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
api/go.mod
Now that IP and subnet are ip and cidr postgres types, we'll need to migrate the columns in case assisted-service is upgraded from a prior version.
Now that the DB validation for CIDRs is more strict, the business logic should add these validations ahead of the DB errors to prevent them.
CrystalChun
force-pushed
the
dual-stack-ip-order
branch
from
cd089fc to
da453a4
Compare
Contributor
Author
|
/retest-required |
|
@CrystalChun: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, dual-stack network ordering was not guaranteed when fetching a cluster from the DB since the fields are
text fields.
In order to guarantee the order, every fetch for the cluster from the db will add a postgres clause for the network
subtables.
The postgres clause gets the ip family of the network ip addresses or cidrs, and will return them in order based on the primary_ip_stack column for that cluster.
Clause that gets added to every network table (
api_vips,ingress_vips,cluster_networks,machine_networks, andservice_networks) on select/get:Includes the following DB column type changes:
textinetapi_vipsandingress_vipstextcidrcluster_networks,machine_networks, andservice_networkstextintList all the issues related to this PR
What environments does this code impact?
How was this code tested?
Manual Testing
See
PR Testingsection of doc https://docs.google.com/document/d/1fzQQLUrJwmyU_XvSrizoi-AVrcvIxVGuAxcd7kIZb2Q/edit?usp=sharingChecklist
docs, README, etc)Reviewers Checklist