Skip to content
This repository was archived by the owner on Mar 25, 2024. It is now read-only.

[2.4] [Fix] Updated api bool fields with default=true to *bool, to be usable from client#1083

Merged
dramich merged 2 commits into
rancher:release/v2.4from
rawmind0:apiBool
May 28, 2020
Merged

[2.4] [Fix] Updated api bool fields with default=true to *bool, to be usable from client#1083
dramich merged 2 commits into
rancher:release/v2.4from
rawmind0:apiBool

Conversation

@rawmind0
Copy link
Copy Markdown
Contributor

@rawmind0 rawmind0 commented Feb 10, 2020

This PR updates api bool fields with default=true to *bool, to be usable from client:

  • IgnoreDockerVersion on rke_types
  • UseInternalIPAddress on NodeCommonParams and IgnoreDaemonSets on NodeDrainInput on machine_types
  • Compress on FluentForwarderConfig on logging_types
  • TLS on SMTPConfig on alerting_types

This is related with golang Marshal function from encoding/json that omits false bool if omitempty is set, golang/go#13284 (comment)

Fix issues https://github.com/terraform-providers/terraform-provider-rancher2/issues/41, https://github.com/terraform-providers/terraform-provider-rancher2/issues/264, https://github.com/terraform-providers/terraform-provider-rancher2/issues/356 and rancher/rancher#25321

@mrajashree
Copy link
Copy Markdown
Contributor

@rawmind0 this will need changes in rancher and rke because both repos use these fields as bool, so we can't merge this till the other two PRs are also ready for merging

@rawmind0 rawmind0 force-pushed the apiBool branch 2 times, most recently from 994d60e to e155429 Compare May 20, 2020 17:39
@rawmind0 rawmind0 changed the base branch from master to release/v2.4 May 20, 2020 17:39
@rawmind0 rawmind0 changed the title [Fix] Updated api bool fields with default=true to *bool, to be usable from client [2.4] [Fix] Updated api bool fields with default=true to *bool, to be usable from client May 20, 2020
@rawmind0
Copy link
Copy Markdown
Contributor Author

rawmind0 commented May 20, 2020

@mrajashree created related PRs rancher/rancher#27169 and rancher/rke#2064

@mrajashree
Copy link
Copy Markdown
Contributor

@rawmind0 thanks, can we also test this doesn't break things on upgrade from 2.3 branch?

@rawmind0
Copy link
Copy Markdown
Contributor Author

PR rebased to address conflicts.

Upgrade has been tested successfully from rancher v2.3.6, to v2.4.2, to this changes on 2.4 branch.

mrajashree
mrajashree previously approved these changes May 21, 2020
@mrajashree mrajashree requested a review from dramich May 21, 2020 16:27
dramich
dramich previously approved these changes May 26, 2020
@rawmind0 rawmind0 dismissed stale reviews from dramich and mrajashree via c5347d8 May 26, 2020 21:18
@maggieliu maggieliu requested a review from mrajashree May 27, 2020 00:40
mrajashree
mrajashree previously approved these changes May 28, 2020
@mrajashree mrajashree requested a review from dramich May 28, 2020 17:49
Comment thread go.mod Outdated
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants