Fix tinkerbell IP in hegel urls controller #7574
Conversation
eks-distro-bot
added
the
size/M
mitalipaygude
added
do-not-merge/hold
and removed
size/M
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7574 +/- ##
==========================================
+ Coverage 73.48% 73.55% +0.07%
==========================================
Files 579 580 +1
Lines 36357 36664 +307
==========================================
+ Hits 26718 26970 +252
- Misses 7875 7916 +41
- Partials 1764 1778 +14 ☔ View full report in Codecov by Sentry. |
eks-distro-bot
added
the
size/M
|
/approve |
mitalipaygude
force-pushed
the
tink_ip_fix
branch
from
ad7f546 to
bc26f1f
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhay-krishna, mitalipaygude 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 |
| } | ||
|
|
||
| // HasTinkerbellIPAnnotation returns the tinkerbell IP value if the annotation exists. | ||
| func (c *Cluster) HasTinkerbellIPAnnotation() (string, bool) { |
There was a problem hiding this comment.
You might consider not using the bool in the return. The alternative is for consumers to check if the returned string is empty or not. The bool adds extra checking that doesn't feel necessary. If you do go with this idea, you might then rethink the name of the method. Maybe just remove the Has, so its just TinkerbellIPAnnotation.
| @@ -38,6 +39,11 @@ func (s *installEksaComponentsOnBootstrapTask) Checkpoint() *task.CompletedTask | |||
| type installEksaComponentsOnWorkloadTask struct{} | |||
|
|
|||
| func (s *installEksaComponentsOnWorkloadTask) Run(ctx context.Context, commandContext *task.CommandContext) task.Task { | |||
| if commandContext.ClusterSpec.Cluster.Spec.DatacenterRef.Kind == v1alpha1.TinkerbellDatacenterKind { | |||
There was a problem hiding this comment.
a code comment here around why this is the only place we remove the annotation would be very helpful to future us.
| @@ -28,6 +28,8 @@ func (p *Provider) BootstrapClusterOpts(_ *cluster.Spec) ([]bootstrapper.Bootstr | |||
| func (p *Provider) PreCAPIInstallOnBootstrap(ctx context.Context, cluster *types.Cluster, clusterSpec *cluster.Spec) error { | |||
| logger.V(4).Info("Installing Tinkerbell stack on bootstrap cluster") | |||
|
|
|||
| logger.V(4).Info("Adding annotation for tinkerbell ip on bootstrap cluster") | |||
| clusterSpec.Cluster.AddTinkerbellIPAnnotation(p.tinkerbellIP) | |||
There was a problem hiding this comment.
a code comment here are why we need to add this annotation will be very helpful for future us.
Issue #, if available:
Description of changes:
Fix tinkerbell IP in hegel urls controller
Testing (if applicable):
Documentation added/planned (if applicable):
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.