Skip to content

Commit

Permalink
MGMT-15070: Unable to change machine-network with dual stack (openshi…
Browse files Browse the repository at this point in the history
…ft#5349)

When working with dual stack, and there is an attempt to update the
machine networks only, they are not updated.  This change fixes it.
  • Loading branch information
ori-amizur authored and danielerez committed Oct 15, 2023
1 parent efe68cc commit 61cf4c2
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
4 changes: 2 additions & 2 deletions internal/bminventory/inventory.go
Expand Up @@ -2112,6 +2112,7 @@ func (b *bareMetalInventory) updateNonDhcpNetworkParams(updates map[string]inter
// We need to do it because updateNonDhcpNetworkParams is called before updateNetworks, so
// inside this function here cluster object has still values before applying requested changes.
targetConfiguration := common.Cluster{}
targetConfiguration.ID = cluster.ID
targetConfiguration.ClusterNetworks = cluster.ClusterNetworks
targetConfiguration.ServiceNetworks = cluster.ServiceNetworks
targetConfiguration.MachineNetworks = cluster.MachineNetworks
Expand Down Expand Up @@ -2171,7 +2172,6 @@ func (b *bareMetalInventory) updateNonDhcpNetworkParams(updates map[string]inter
// require that user explicitly provides all the Machine Networks.
if reqDualStack {
if params.ClusterUpdateParams.MachineNetworks != nil {
cluster.MachineNetworks = params.ClusterUpdateParams.MachineNetworks
primaryMachineNetworkCidr = string(params.ClusterUpdateParams.MachineNetworks[0].Cidr)
} else {
primaryMachineNetworkCidr = network.GetPrimaryMachineCidrForUserManagedNetwork(cluster, log)
Expand All @@ -2182,7 +2182,7 @@ func (b *bareMetalInventory) updateNonDhcpNetworkParams(updates map[string]inter
log.WithError(err).Warnf("Verify dual-stack machine networks")
return common.NewApiError(http.StatusBadRequest, err)
}
secondaryMachineNetworkCidr, err = network.GetSecondaryMachineCidr(cluster)
secondaryMachineNetworkCidr, err = network.GetSecondaryMachineCidr(&targetConfiguration)
if err != nil {
return common.NewApiError(http.StatusBadRequest, err)
}
Expand Down
33 changes: 30 additions & 3 deletions internal/bminventory/inventory_test.go
Expand Up @@ -17283,11 +17283,14 @@ var _ = Describe("Dual-stack cluster", func() {
})

Context("Update cluster", func() {
var params installer.V2UpdateClusterParams
var (
params installer.V2UpdateClusterParams
clusterID strfmt.UUID
)

BeforeEach(func() {
mockUsageReports()
clusterID := strfmt.UUID(uuid.New().String())
clusterID = strfmt.UUID(uuid.New().String())
err := db.Create(&common.Cluster{Cluster: models.Cluster{
ID: &clusterID,
OpenshiftVersion: common.TestDefaultConfig.OpenShiftVersion,
Expand Down Expand Up @@ -17339,8 +17342,32 @@ var _ = Describe("Dual-stack cluster", func() {
verifyApiErrorString(reply, http.StatusBadRequest, "Expected 2 machine networks, found 1")
})
})
It("update dual machine networks", func() {
By("setup dual stack networking")
cluster := &common.Cluster{
Cluster: models.Cluster{
ID: &clusterID,
},
}
mockClusterApi.EXPECT().VerifyClusterUpdatability(createClusterIdMatcher(cluster)).Return(nil).Times(2)
params.ClusterUpdateParams.MachineNetworks = common.TestDualStackNetworking.MachineNetworks
params.ClusterUpdateParams.ServiceNetworks = common.TestDualStackNetworking.ServiceNetworks
params.ClusterUpdateParams.ClusterNetworks = common.TestDualStackNetworking.ClusterNetworks
cls, err := bm.UpdateClusterNonInteractive(ctx, params)
Expect(err).ToNot(HaveOccurred())
Expect(cls.MachineNetworks).To(HaveLen(2))
for _, m := range common.TestDualStackNetworking.MachineNetworks {
Expect(cls.MachineNetworks).To(ContainElement(&models.MachineNetwork{ClusterID: clusterID, Cidr: m.Cidr}))
}
By("update machine networks")
params.ClusterUpdateParams.MachineNetworks = append([]*models.MachineNetwork{}, common.TestDualStackNetworking.MachineNetworks...)
params.ClusterUpdateParams.MachineNetworks[1] = &models.MachineNetwork{ClusterID: clusterID, Cidr: "3001:db8::/120"}
cls, err = bm.UpdateClusterNonInteractive(ctx, params)
Expect(err).ToNot(HaveOccurred())
Expect(cls.MachineNetworks).To(HaveLen(2))
Expect(cls.MachineNetworks).To(ContainElement(&models.MachineNetwork{ClusterID: clusterID, Cidr: "3001:db8::/120"}))
})
})

})

var _ = Describe("GetCredentials", func() {
Expand Down

0 comments on commit 61cf4c2

Please sign in to comment.