chore: cleanup dead code #4053
chore: cleanup dead code #4053
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4053 +/- ##
==========================================
- Coverage 73.22% 73.21% -0.01%
==========================================
Files 135 135
Lines 20640 20540 -100
==========================================
- Hits 15113 15039 -74
+ Misses 4550 4527 -23
+ Partials 977 974 -3
Continue to review full report at Codecov.
|
| @@ -1106,20 +1010,6 @@ func (p *Properties) GetClusterID() string { | |||
| return p.ClusterID | |||
| } | |||
|
|
|||
| // GetClusterMetadata returns a instance of the struct type api.ClusterMetadata. | |||
| func (p *Properties) GetClusterMetadata() *ClusterMetadata { | |||
There was a problem hiding this comment.
This wasn't used anywhere outside the unit test, eh? So much cruft!
| @@ -535,15 +479,6 @@ type CustomFile struct { | |||
| Dest string `json:"dest,omitempty"` | |||
| } | |||
|
|
|||
| // BootstrapProfile represents the definition of the DCOS bootstrap node used to deploy the cluster | |||
There was a problem hiding this comment.
Nice! I must have missed these structs in #3965.
| return p.OrchestratorProfile.KubernetesConfig.IsIPMasqAgentDisabled() | ||
| } | ||
| return false | ||
| return p.OrchestratorProfile != nil && |
There was a problem hiding this comment.
Alternate implementation: return !p.IsIPMasqAgentEnabled().
There was a problem hiding this comment.
so this is a confusing one that I stumbled around as well
Basically there's a race condition between kubelet and addons defaults enforcement, and they depend on each other for this particular config. So what we want to confirm is that the addon is explicitly disabled. Whereas the "is enabled" foo will return false when it's empty/nil/whatever, which is misleading, because if you consider that authoritative and then the addons default foo sets it to true later in the code flow, you're in trouble.
Luckily we have UT to tell this story, and when I did the obvious thing that you suggested they complained!
There was a problem hiding this comment.
Ah, I was wondering if it was more of a tri-state boolean but I made the naive comment anyway. I don't remember these details for very long apparently. :-)
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis, mboersma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Reason for Change:
This PR removes dead code, the result of an audit of
pkg/api/types.go. Plenty of indirection happened and various parts of the codebase were pruned, but the main story was culling through the type library for no-longer relevant stuffs.Issue Fixed:
Credit Where Due:
Does this change contain code from or inspired by another project?
If "Yes," did you notify that project's maintainers and provide attribution?
Requirements:
Notes: