Review

Author: lukasfrank

LICENSES/Apache-2.0.txt

@@ -44,11 +44,11 @@ TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION
 
 5. Submission of Contributions. Unless You explicitly state otherwise, any Contribution intentionally submitted for inclusion in the Work by You to the Licensor shall be under the terms and conditions of this License, without any additional terms or conditions. Notwithstanding the above, nothing herein shall supersede or modify the terms of any separate license agreement you may have executed with Licensor regarding such Contributions.
 
-6. Trademarks. This License does not grant permission to use the trade names, trademarks, service marks, or product names of the Licensor, except as requested for reasonable and customary use in describing the origin of the Work and reproducing the content of the NOTICE file.
+6. Trademarks. This License does not grant permission to use the trade names, trademarks, service marks, or product names of the Licensor, except as required for reasonable and customary use in describing the origin of the Work and reproducing the content of the NOTICE file.
 
-7. Disclaimer of Warranty. Unless requested by applicable law or agreed to in writing, Licensor provides the Work (and each Contributor provides its Contributions) on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied, including, without limitation, any warranties or conditions of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A PARTICULAR PURPOSE. You are solely responsible for determining the appropriateness of using or redistributing the Work and assume any risks associated with Your exercise of permissions under this License.
+7. Disclaimer of Warranty. Unless required by applicable law or agreed to in writing, Licensor provides the Work (and each Contributor provides its Contributions) on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied, including, without limitation, any warranties or conditions of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A PARTICULAR PURPOSE. You are solely responsible for determining the appropriateness of using or redistributing the Work and assume any risks associated with Your exercise of permissions under this License.
 
-8. Limitation of Liability. In no event and under no legal theory, whether in tort (including negligence), contract, or otherwise, unless requested by applicable law (such as deliberate and grossly negligent acts) or agreed to in writing, shall any Contributor be liable to You for damages, including any direct, indirect, special, incidental, or consequential damages of any character arising as a result of this License or out of the use or inability to use the Work (including but not limited to damages for loss of goodwill, work stoppage, computer failure or malfunction, or any and all other commercial damages or losses), even if such Contributor has been advised of the possibility of such damages.
+8. Limitation of Liability. In no event and under no legal theory, whether in tort (including negligence), contract, or otherwise, unless required by applicable law (such as deliberate and grossly negligent acts) or agreed to in writing, shall any Contributor be liable to You for damages, including any direct, indirect, special, incidental, or consequential damages of any character arising as a result of this License or out of the use or inability to use the Work (including but not limited to damages for loss of goodwill, work stoppage, computer failure or malfunction, or any and all other commercial damages or losses), even if such Contributor has been advised of the possibility of such damages.
 
 9. Accepting Warranty or Additional Liability. While redistributing the Work or Derivative Works thereof, You may choose to offer, and charge a fee for, acceptance of support, warranty, indemnity, or other liability obligations and/or rights consistent with this License. However, in accepting such obligations, You may act only on Your own behalf and on Your sole responsibility, not on behalf of any other Contributor, and only if You agree to indemnify, defend, and hold each Contributor harmless for any liability incurred by, or claims asserted against, such Contributor by reason of your accepting any such warranty or additional liability.
 
@@ -66,7 +66,7 @@ You may obtain a copy of the License at
 
 http://www.apache.org/licenses/LICENSE-2.0
 
-Unless requested by applicable law or agreed to in writing, software
+Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and

claimutils/claim/claimer.go

@@ -28,8 +28,9 @@ type Claimer interface {
 	Start(ctx context.Context) error
 }
 
-func NewResourceClaimer(plugins ...Plugin) (*claimer, error) {
+func NewResourceClaimer(log logr.Logger, plugins ...Plugin) (*claimer, error) {
 	c := claimer{
+		log:       log,
 		plugins:   map[string]Plugin{},
 		toClaim:   make(chan claimReq, 1),
 		toRelease: make(chan releaseReq, 1),

claimutils/claim/claimer_test.go

@@ -29,6 +29,7 @@ var _ = Describe("Resource Claimer", func() {
 	It("should claim composite resources", func(ctx SpecContext) {
 		By("init plugin")
 		resourceClaimer, err := claim.NewResourceClaimer(
+			log.FromContext(ctx),
 			gpu.NewGPUClaimPlugin(log.FromContext(ctx), "nvidia.com/gpu", &mockReader{
 				devices: []pci.Address{
 					{},
@@ -39,15 +40,20 @@ var _ = Describe("Resource Claimer", func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		innerCtx, cancel := context.WithCancel(ctx)
-		defer cancel()
-
-		// start the Runnable in the background
 		errCh := make(chan error, 1)
+		defer cancel()
 		go func() {
 			defer GinkgoRecover()
 			errCh <- resourceClaimer.Start(innerCtx)
 		}()
 
+		DeferCleanup(func() {
+			cancel()
+			var startErr error
+			Eventually(errCh).Should(Receive(&startErr))
+			Expect(startErr).To(Succeed())
+		})
+
 		By("failing if not not existing resource is claimed")
 		var resourceClaim claim.Claims
 		Eventually(func() error {

claimutils/gpu/gpu.go

@@ -138,12 +138,12 @@ func (g *gpuClaimPlugin) Init() error {
 	for _, pciDevice := range g.preClaimed {
 		if _, ok := g.devices[pciDevice]; !ok {
 			g.log.V(2).Info("Not discovered pre-claimed pci address", "pciAddress", pciDevice)
-
+			continue
 		}
 
 		g.log.V(2).Info("Set device to claimed", "pciAddress", pciDevice)
 		g.devices[pciDevice] = ClaimStatusClaimed
-		continue
+
 	}
 
 	return nil

claimutils/gpu/gpu_test.go

@@ -146,7 +146,7 @@ var _ = Describe("GPU Claimer", func() {
 		Expect(ociAddress1.PCIAddresses()[0]).NotTo(Equal(ociAddress2.PCIAddresses()[0]))
 	})
 
-	It("should claim different devices", func(ctx SpecContext) {
+	It("should handle zero-quantity claims", func(ctx SpecContext) {
 		By("init plugin")
 		plugin := gpu.NewGPUClaimPlugin(log.FromContext(ctx), "test-plugin", &MockReader{
 			devices: []pci.Address{

claimutils/pci/reader_test.go

@@ -59,7 +59,7 @@ func writeFakePCIDevice(t *testing.T, sysRoot, id string, vals map[string]string
 	}
 }
 
-func TestGPUClaimer_InitCorrect(t *testing.T) {
+func TestPCIReader_ReadFilters(t *testing.T) {
 	tmpDir := t.TempDir()
 
 	// matching device 1