-
Notifications
You must be signed in to change notification settings - Fork 137
effective-creation-timeout & new metrics for use by dependency-watchdog #1104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 6 commits
efbc070
494e78b
d6d57f5
4fcf8d2
b5420f3
c645b19
3a5fe51
535bcd2
dfd5870
0efcf8a
e3ce5cb
97b3a51
3284907
0d8115d
fd926a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ import ( | |
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/gardener/machine-controller-manager/pkg/util/provider/metrics" | ||
| corev1 "k8s.io/api/core/v1" | ||
| apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
|
|
@@ -333,6 +334,7 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque | |
| machineName = createMachineRequest.Machine.Name | ||
| uninitializedMachine = false | ||
| addresses = sets.New[corev1.NodeAddress]() | ||
| createDuration time.Duration | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, but could you move this up to the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't get the comment. This is zero value not
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, got it - moved it upward. |
||
| ) | ||
| c.markCreationProcessing(machine) | ||
| defer func() { | ||
|
|
@@ -392,7 +394,9 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque | |
| providerID = createMachineResponse.ProviderID | ||
| addresses.Insert(createMachineResponse.Addresses...) | ||
| // Creation was successful | ||
| klog.V(2).Infof("Created new VM for machine: %q with ProviderID: %q and backing node: %q", machine.Name, providerID, nodeName) | ||
| createDuration = time.Since(machine.CreationTimestamp.Time) | ||
| klog.V(2).Infof("Created new VM for machine: %q with ProviderID: %q and backing node: %q, createDuration: %s", machine.Name, providerID, nodeName, createDuration) | ||
| metrics.UpdateMetricsForMachineDurations(machine, metrics.MachineDurations{Create: createDuration}) | ||
|
|
||
| if c.nodeLister != nil { | ||
| // If a node obj already exists by the same nodeName, treat it as a stale node and trigger machine deletion. | ||
|
|
@@ -489,12 +493,19 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque | |
| } | ||
| //initialize VM if not initialized | ||
| if uninitializedMachine { | ||
| var retryPeriod machineutils.RetryPeriod | ||
| var initResponse *driver.InitializeMachineResponse | ||
| var ( | ||
| retryPeriod machineutils.RetryPeriod | ||
| initResponse *driver.InitializeMachineResponse | ||
| initializeBeginTime = time.Now() | ||
| initializeDuration time.Duration | ||
| ) | ||
| initResponse, retryPeriod, err = c.initializeMachine(ctx, clone, createMachineRequest.MachineClass, createMachineRequest.Secret) | ||
| if err != nil { | ||
| return retryPeriod, err | ||
| } | ||
| initializeDuration = time.Since(initializeBeginTime) | ||
| klog.V(2).Infof("Machine %q was initialized in %q", machine.Name, initializeDuration) | ||
|
elankath marked this conversation as resolved.
Outdated
|
||
| metrics.UpdateMetricsForMachineDurations(machine, metrics.MachineDurations{Initialize: initializeDuration}) | ||
|
|
||
| if c.targetCoreClient == nil { | ||
| // persist addresses from the InitializeMachine and CreateMachine responses | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this result in all parent object annotations to now be present on the machine object? if that is the case then there might be additional unnecessary annotations present on the machine object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. I took a look at MCD annotations copied to MCS annotations. I assumed we copy everything there and therefore we should be symmetric when doing MCS->MC. but apparently in
copyMachineDeploymentAnnotationsToMachineSetwe have askipCopyAnnotationwhich checks whether annotation should be copied. I will reuse this.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind - re-use involves lot of refactoring due to different packages. For now, will just copy one annotation. Refactoring annotations for DRY makes too many changes.