Conversation
| @@ -0,0 +1,19 @@ | |||
|
|
|||
There was a problem hiding this comment.
Can we move this into a tftp directory in the helm chart as tftp/service.yaml?
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: {{ include "network-operator.fullname" . }}-tftp |
There was a problem hiding this comment.
This helper does not exist
template: no template "network-operator.fullname" associated with template "gotpl"
| metadata: | ||
| name: {{ include "network-operator.fullname" . }}-tftp | ||
| labels: | ||
| {{- include "network-operator.labels" . | nindent 8 }} |
There was a problem hiding this comment.
| {{- include "network-operator.labels" . | nindent 8 }} | |
| {{- include "chart.labels" . | nindent 4 }} |
| kind: Service | ||
| metadata: | ||
| name: {{ include "network-operator.fullname" . }}-tftp | ||
| labels: |
There was a problem hiding this comment.
| labels: | |
| namespace: {{ .Release.Namespace }} | |
| labels: |
| {{- range .Values.controllerManager.container.args }} | ||
| - {{ . }} | ||
| {{- end }} | ||
| {{- if .Values.tftp.enabled }} |
There was a problem hiding this comment.
Please also add these in the https://github.com/ironcore-dev/network-operator/blob/main/config/develop/manager_patch.yaml so that the ftpt functionality is enabled by default when running the local development setup.
|
|
||
| // Ensure Device IP label is set for inline services to filter by client IP. | ||
| ip := device.Spec.Endpoint.Address | ||
| if ip != "" { |
There was a problem hiding this comment.
I guess we don't need the empty check here, as this field is required in the API.
| } | ||
|
|
||
| // LookupBySerial finds a Device by status.serialNumber. | ||
| func (i K8sIndex) LookupBySerial(serial string) (DeviceInfo, bool) { |
There was a problem hiding this comment.
I think these functions are not needed, as you can simply use a label selector to efficiently query for the devices:
selector := labels.SelectorFromSet(labels.Set{v1alpha1.DeviceSerialLabel: serial})
list := new(corev1.DeviceList)
if err := i.Reader.List(ctx, list, client.MatchingLabelsSelector{Selector: selector}); err != nil {
return err
}which can be done directly from the tftp.Server without the need for this K8sIndex.
internal/tftp/server.go
Outdated
| if strings.HasPrefix(fn, "serial-") { | ||
| fn = strings.TrimPrefix(fn, "serial-") | ||
| } | ||
| if i := strings.IndexByte(fn, '.'); i >= 0 { |
There was a problem hiding this comment.
With this, we don't need the TrimSuffix, do we?
internal/tftp/server.go
Outdated
| } | ||
|
|
||
| srv := tftp.NewServer(readHandler, writeHandler) | ||
| // Optional: srv.SetTimeout(time.Duration(s.timeoutSeconds) * time.Second) |
There was a problem hiding this comment.
Can we remove these optional comments?
Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. |
hardikdr
added
the
area/metal-automation
3 participants
issue ticket: https://github.wdf.sap.corp/sap-cloud-infrastructure/neutron-issues/issues/217
Summary
Device.spec.provisioning.bootScriptregardless of requested path.--verify-tftp-clientenforces serial + IP match.DeviceIPLabeland reconcile logic to populate it.Key changes
internal/tftp/server.go,internal/tftp/index.gocmd/main.goapi/core/v1alpha1/groupversion_info.go,internal/controller/core/device_controller.goconfig/overlay/netop-dev-minmin-noglobaldocs/TFTP_FEATURE.md,docs/TESTING_TFTP.mdHow to test (smoke)
Test result
Notes / risks