Skip to content

Conversation

@MujibAzizi
Copy link
Contributor

@MujibAzizi MujibAzizi commented Dec 9, 2020

This PR adds support for labels to more classes which extend the K8sResource class.

Right now, calling setLabels on, for example, K8sService leads to the HasAttributes's __call method being used which adds the labels as a top level key rather than as a child of metadata. To correct the behaviour of this, the HasLabels must be added to all classes.

Classes to add HasLabels, tests and update documentation for:

  • K8sClusterRoleBinding
  • K8sConfigMap
  • K8sHorizontalPodAutoscaler
  • K8sIngress
  • K8sNamespace
  • K8sPersistentVolume
  • K8sPersistentVolumeClaim
  • K8sRole
  • K8sRoleBinding
  • K8sScale Scale is not an Kubernetes resource and thus I will not add HasLabels to it.
  • K8sSecret
  • K8sService
  • K8sServiceAccount
  • K8sStorageClass

Closes #52

@codecov
Copy link

codecov bot commented Dec 9, 2020

Codecov Report

Merging #53 (48b2e3d) into master (04c5555) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #53   +/-   ##
=========================================
  Coverage     92.34%   92.34%           
  Complexity      497      497           
=========================================
  Files            54       54           
  Lines          1202     1202           
=========================================
  Hits           1110     1110           
  Misses           92       92           
Impacted Files Coverage Δ Complexity Δ
src/Kinds/K8sConfigMap.php 100.00% <ø> (ø) 5.00 <0.00> (ø)
src/Kinds/K8sHorizontalPodAutoscaler.php 100.00% <ø> (ø) 14.00 <0.00> (ø)
src/Kinds/K8sIngress.php 100.00% <ø> (ø) 5.00 <0.00> (ø)
src/Kinds/K8sNamespace.php 100.00% <ø> (ø) 2.00 <0.00> (ø)
src/Kinds/K8sPersistentVolume.php 100.00% <ø> (ø) 5.00 <0.00> (ø)
src/Kinds/K8sPersistentVolumeClaim.php 100.00% <ø> (ø) 4.00 <0.00> (ø)
src/Kinds/K8sRoleBinding.php 100.00% <ø> (ø) 2.00 <0.00> (ø)
src/Kinds/K8sSecret.php 100.00% <ø> (ø) 9.00 <0.00> (ø)
src/Kinds/K8sService.php 100.00% <ø> (ø) 5.00 <0.00> (ø)
src/Kinds/K8sServiceAccount.php 100.00% <ø> (ø) 10.00 <0.00> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04c5555...48b2e3d. Read the comment docs.

@rennokki
Copy link
Member

rennokki commented Dec 9, 2020

Interestingly enough, I think there are more resources in this situation. 🤔

@MujibAzizi
Copy link
Contributor Author

I will be adding it to all resources, and if that works out we can discuss to move HasLabels to K8sResource.

If you run the tests locally, does it work for you?

@MujibAzizi
Copy link
Contributor Author

MujibAzizi commented Dec 10, 2020

FYI: I am going to switch to using Minikube, Docker for Mac has different defaults.

For example; there is no storageclass called standard available. To fix this is not in scope of this PR.

@rennokki
Copy link
Member

@MujibAzizi I use Minikube for testing, maybe that's what the testing is biased towards it.

If all resources accept Labels, then it can be moved to the K8sResource. However, I think it's not the same as the annotations.

@MujibAzizi MujibAzizi force-pushed the hotfix/add-labels-to-k8s-resources branch from 7d2370f to e0d37e8 Compare December 10, 2020 16:11
@MujibAzizi
Copy link
Contributor Author

@rennokki I've finished adding HasLabels to the required classes and added the tests. Also, I've added some generic documentation.

I'm marking the PR as Ready for Review.

@MujibAzizi MujibAzizi marked this pull request as ready for review December 11, 2020 14:43
@rennokki rennokki merged commit aac3690 into renoki-co:master Dec 11, 2020
@rennokki
Copy link
Member

Thank you for your contribution. ❤️

@MujibAzizi
Copy link
Contributor Author

Thank you for your contribution. ❤️

My pleasure! Thank you for merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Applying HasLabels trait to all children of K8sResource

2 participants