Unit test tooling

.github/workflows/run_unit_tests.yml

@@ -0,0 +1,30 @@
+name: Run Unit Tests
+on:
+  pull_request:
+    types: [opened, synchronize, reopened]
+    branches:
+      - ci_dev
+      - ci_prod
+jobs:
+  Golang-Tests:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v2
+      - name: Run unit tests
+        run: |
+          cd ${{ github.workspace }}
+          ./test/unit-tests/run_go_tests.sh
+  Ruby-Tests:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v2
+      - name: install fluent
+        run: |
+          sudo gem install fluentd -v "1.12.2" --no-document
+          sudo fluentd --setup ./fluent
+      - name: Run unit tests
+        run: |
+          cd ${{ github.workspace }}
+          ./test/unit-tests/run_ruby_tests.sh

.gitignore

@@ -26,3 +26,7 @@ intermediate
 kubernetes/linux/Linux_ULINUX_1.0_x64_64_Release
 # ignore generated .h files for go
 source/plugins/go/src/*.h
+*_mock.go
+*_log.txt
+*.log
+*.byebug_history

Dev Guide.md

@@ -0,0 +1,125 @@
+# Dev Guide
+
+More advanced information needed to develop or build the docker provider will live here
+
+<!-- TODO: eventually move dev info from README.md to here-->
+
+## Testing
+Last updated 8/18/2021
+
+To run all unit tests run the commands `test/unit-tests/run_go_tests.sh` and `test/unit-tests/run_ruby_tests.sh`
+
+#### Conventions:
+1. Unit tests should go in their own file, but in the same folder as the source code their testing. For example, the tests for `in_kube_nodes.rb` are in `in_kube_nodes_test.rb`. Both files are in the folder `source/plugin/ruby`.
+
+### Ruby
+Sample tests are provided in [in_kube_nodes_test.rb](source/plugin/ruby/in_kube_nodes_test.rb). They are meant to demo the tooling used for unit tests (as opposed to being comprehensive tests). Basic techniques like mocking are demonstrated there.
+
+#### Conventions:
+1. When modifying a fluentd plugin for unit testing, any mocked classes (like KubernetesApiClient, applicationInsightsUtility, env, etc.) should be passed in as optional arguments of initialize. For example:
+```
+    def initialize
+      super
+```
+would be turned into
+```
+    def initialize (kubernetesApiClient=nil, applicationInsightsUtility=nil, extensionUtils=nil, env=nil)
+      super()
+```
+
+2. Having end-to-end tests of all fluentd plugins is a longshot. We care more about unit testing smaller blocks of functionality (like all the helper functions in KubeNodeInventory.rb). Unit tests for fluentd plugins are not expected.
+
+### Golang
+
+Since golang is statically compiled, mocking requires a lot more work than in ruby. Sample tests are provided in [utils_test.go](source/plugin/go/src/utils_test.go) and [extension_test.go](source/plugin/go/src/extension/extension_test.go). Again, they are meant to demo the tooling used for unit tests (as opposed to being comprehensive tests). Basic techniques like mocking are demonstrated there.
+
+#### Mocking:
+Mocks are generated with gomock (mockgen). 
+* Mock files should be called *_mock.go (socket_writer.go => socket_writer_mock.go)
+* Mocks should not be checked in to git. (they have been added to the .gitignore)
+* The command to generate mock files should go in a `//go:generate` comment at the top of the mocked file (see [socket_writer.go](source/plugin/go/src/extension/socket_writer.go) for an example). This way mocks can be generated by the unit test script.
+* Mocks also go in the same folder as the mocked files. This is unfortunate, but necessary to avoid circular package dependencies (anyone else feel free to figure out how to move mocks to a separate folder)
+
+Using mocks is also a little tricky. In order to mock functions in a package with gomock, they must be converted to reciever methods of a struct. This way the struct can be swapped out at runtime to change which implementaions of a method are called. See the example below:
+
+```
+// declare all functions to be mocked in this interface
+type registrationPreCheckerInterface interface {
+	FUT(string) bool
+}
+
+// Create a struct which implements the above interface
+type regPreCheck struct{}
+
+func (r regPreCheck) FUT(email string) bool {
+	fmt.Println("real FUT() called")
+	return true
+}
+
+// Create a global variable and assign it to the struct
+var regPreCondVar registrationPreCheckerInterface
+
+func init() {
+	regPreCondVar = regPreCheck{}
+}
+```
+
+Now any code wishing to call FUT() will call `regPreCondVar.FUT("")`
+
+A unit test can substitute its own implementaion of FUT() like so
+
+```
+// This will hold the mock of FUT we want to substitute
+var FUTMock func(email string) bool
+
+// create a new struct which implements the earlier interface
+type regPreCheckMock struct{}
+
+func (u regPreCheckMock) FUT(email string) bool {
+	return FUTMock(email)
+}
+```
+
+Everything is set up. Now a unit test can substitute in a mock like so:
+
+```
+func someUnitTest() {
+    // This will call the actual implementaion of FUT()
+	regPreCondVar.FUT("")
+
+    // Now the test creates another struct to substitue. After this like all calls to FUT() will be diverted
+	regPreCondVar = regPreCheckMock{}
+
+    // substute another function to run instead of FUT()
+	FUTMock = func(email string) bool {
+		fmt.Println("FUT 1 called")
+		return false
+	}
+    // This will call the function defined right above
+	regPreCondVar.FUT("")
+
+    // We can substitue another implementation
+	FUTMock = func(email string) bool {
+		fmt.Println("FUT 2 called")
+		return false
+	}
+	regPreCondVar.FUT("")
+
+    // put the old behavior back
+	regPreCondVar = regPreCheck{}
+    // this will call the actual implementation of FUT()
+	regPreCondVar.FUT("")
+
+}
+```
+
+A concrete example of this can be found in [socket_writer.go](source/plugin/go/src/extension/socket_writer.go) and [extension_test.go](source/plugin/go/src/extension/extension_test.go). Again, if anybody has a better way feel free to update this guide.
+
+
+
+A simpler way to test a specific function is to write wrapper functions. Test code calls the inner function (ReadFileContentsImpl) and product code calls the wrapper function (ReadFileContents). The wrapper function provides any outside state which a unit test would want to control (like a function to read a file). This option makes product code more verbose, but probably easier to read too. Either way is acceptable.
+```
+func ReadFileContents(fullPathToFileName string) (string, error) {
+	return ReadFileContentsImpl(fullPathToFileName, ioutil.ReadFile)
+}
+```

build/linux/installer/datafiles/base_container.data

@@ -148,10 +148,10 @@ MAINTAINER:              'Microsoft Corporation'
 /etc/fluent/plugin/MdmMetricsGenerator.rb;                                             source/plugins/ruby/MdmMetricsGenerator.rb;            644; root; root
 /etc/fluent/plugin/MdmAlertTemplates.rb;                                               source/plugins/ruby/MdmAlertTemplates.rb;                644; root; root
 
-/etc/fluent/plugin/omslog.rb;                                                           source/plugins/utils/omslog.rb; 644; root; root
-/etc/fluent/plugin/oms_common.rb;                                                       source/plugins/utils/oms_common.rb; 644; root; root
-/etc/fluent/plugin/extension.rb;                                                        source/plugins/utils/extension.rb; 644; root; root
-/etc/fluent/plugin/extension_utils.rb;                                                  source/plugins/utils/extension_utils.rb; 644; root; root
+/etc/fluent/plugin/omslog.rb;                                                           source/plugins/ruby/omslog.rb; 644; root; root
+/etc/fluent/plugin/oms_common.rb;                                                       source/plugins/ruby/oms_common.rb; 644; root; root
+/etc/fluent/plugin/extension.rb;                                                        source/plugins/ruby/extension.rb; 644; root; root
+/etc/fluent/plugin/extension_utils.rb;                                                  source/plugins/ruby/extension_utils.rb; 644; root; root
 
 
 /etc/fluent/kube.conf;			                                                          build/linux/installer/conf/kube.conf;                  644; root; root

build/windows/Makefile.ps1

@@ -183,11 +183,7 @@ Write-Host("successfully copied installer files conf and scripts from :" + $inst
 $rubyplugindir = Join-Path -Path $rootdir -ChildPath "source\plugins\ruby"
 Write-Host("copying ruby source files from :" + $rubyplugindir + "  to  :" + $publishdir + " ...")
 Copy-Item -Path $rubyplugindir -Destination $publishdir -Recurse -Force
+Get-ChildItem $Path | Where{$_.Name -Match ".*_test\.rb"} | Remove-Item
 Write-Host("successfully copied ruby source files from :" + $rubyplugindir + "  to  :" + $publishdir + " ") -ForegroundColor Green
 
-$utilsplugindir = Join-Path -Path $rootdir -ChildPath "source\plugins\utils"
-Write-Host("copying ruby util files from :" + $utilsplugindir + "  to  :" + $publishdir + " ...")
-Copy-Item -Path $utilsplugindir -Destination $publishdir -Recurse -Force
-Write-Host("successfully copied ruby util files from :" + $utilsplugindir + "  to  :" + $publishdir + " ") -ForegroundColor Green
-
-Set-Location $currentdir
+Set-Location $currentdir

kubernetes/windows/Dockerfile

@@ -71,7 +71,6 @@ COPY ./omsagentwindows/installer/scripts/rubyKeepCertificateAlive/*.rb /etc/flue
 
 #Copy fluentd ruby plugins
 COPY ./omsagentwindows/ruby/ /etc/fluent/plugin/
-COPY ./omsagentwindows/utils/*.rb /etc/fluent/plugin/
 
 ENV AGENT_VERSION ${IMAGE_TAG}
 ENV OS_TYPE "windows"

kubernetes/windows/Dockerfile-dev-image

@@ -33,7 +33,6 @@ COPY ./omsagentwindows/installer/scripts/rubyKeepCertificateAlive/*.rb /etc/flue
 
 #Copy fluentd ruby plugins
 COPY ./omsagentwindows/ruby/ /etc/fluent/plugin/
-COPY ./omsagentwindows/utils/*.rb /etc/fluent/plugin/
 
 ENV AGENT_VERSION ${IMAGE_TAG}
 ENV OS_TYPE "windows"

source/plugins/go/src/extension/extension.go

@@ -1,44 +1,45 @@
 package extension
 
-import (		
+import (
 	"encoding/json"
 	"fmt"
-	"log"	
+	"log"
+	"strings"
 	"sync"
-	"strings"		
-	uuid "github.com/google/uuid"	
+
+	uuid "github.com/google/uuid"
 	"github.com/ugorji/go/codec"
 )
 
 type Extension struct {
 	datatypeStreamIdMap map[string]string
 }
 
-var singleton *Extension 
+var singleton *Extension
 var once sync.Once
 var extensionconfiglock sync.Mutex
 var logger *log.Logger
-var containerType string 
+var containerType string
 
-func GetInstance(flbLogger *log.Logger, containerType string) *Extension {
-    once.Do(func() {
-        singleton = &Extension{make(map[string]string)}
+func GetInstance(flbLogger *log.Logger, containertype string) *Extension {
+	once.Do(func() {
+		singleton = &Extension{make(map[string]string)}
 		flbLogger.Println("Extension Instance created")
-    })
+	})
 	logger = flbLogger
-	containerType = containerType
-    return singleton
+	containerType = containertype
+	return singleton
 }
 
 func (e *Extension) GetOutputStreamId(datatype string) string {
 	extensionconfiglock.Lock()
-	defer extensionconfiglock.Unlock()  	
+	defer extensionconfiglock.Unlock()
 	if len(e.datatypeStreamIdMap) > 0 && e.datatypeStreamIdMap[datatype] != "" {
 		message := fmt.Sprintf("OutputstreamId: %s for the datatype: %s", e.datatypeStreamIdMap[datatype], datatype)
 		logger.Printf(message)
 		return e.datatypeStreamIdMap[datatype]
 	}
-	var err error 
+	var err error
 	e.datatypeStreamIdMap, err = getDataTypeToStreamIdMapping()
 	if err != nil {
 		message := fmt.Sprintf("Error getting datatype to streamid mapping: %s", err.Error())
@@ -54,46 +55,47 @@ func getDataTypeToStreamIdMapping() (map[string]string, error) {
 
 	taggedData := map[string]interface{}{"Request": "AgentTaggedData", "RequestId": guid.String(), "Tag": "ContainerInsights", "Version": "1"}
 	jsonBytes, err := json.Marshal(taggedData)
+	// TODO: this error is unhandled
 
 	var data []byte
-	enc := codec.NewEncoderBytes(&data, new(codec.MsgpackHandle))	
+	enc := codec.NewEncoderBytes(&data, new(codec.MsgpackHandle))
 	if err := enc.Encode(string(jsonBytes)); err != nil {
 		return datatypeOutputStreamMap, err
 	}
-	
-	fs := &FluentSocketWriter{ }
+
+	fs := &FluentSocket{}
 	fs.sockAddress = "/var/run/mdsd/default_fluent.socket"
 	if containerType != "" && strings.Compare(strings.ToLower(containerType), "prometheussidecar") == 0 {
 		fs.sockAddress = fmt.Sprintf("/var/run/mdsd-%s/default_fluent.socket", containerType)
-	}     
-	responseBytes, err := fs.WriteAndRead(data)
-	defer fs.disConnect()
+	}
+	responseBytes, err := FluentSocketWriter.writeAndRead(fs, data)
+	defer FluentSocketWriter.disconnect(fs)
 	logger.Printf("Info::mdsd::Making call to FluentSocket: %s to write and read the config data", fs.sockAddress)
 	if err != nil {
 		return datatypeOutputStreamMap, err
 	}
-	response := string(responseBytes)
+	response := string(responseBytes) // TODO: why is this converted to a string then back into a []byte?
 
 	var responseObjet AgentTaggedDataResponse
 	err = json.Unmarshal([]byte(response), &responseObjet)
-	if err != nil {	
+	if err != nil {
 		logger.Printf("Error::mdsd::Failed to unmarshal config data. Error message: %s", string(err.Error()))
 		return datatypeOutputStreamMap, err
 	}
 
 	var extensionData TaggedData
 	json.Unmarshal([]byte(responseObjet.TaggedData), &extensionData)
 
-	extensionConfigs := extensionData.ExtensionConfigs	
-	logger.Printf("Info::mdsd::build the datatype and streamid map -- start")	
+	extensionConfigs := extensionData.ExtensionConfigs
+	logger.Printf("Info::mdsd::build the datatype and streamid map -- start")
 	for _, extensionConfig := range extensionConfigs {
 		outputStreams := extensionConfig.OutputStreams
 		for dataType, outputStreamID := range outputStreams {
 			logger.Printf("Info::mdsd::datatype: %s, outputstreamId: %s", dataType, outputStreamID)
 			datatypeOutputStreamMap[dataType] = outputStreamID.(string)
-		}	
+		}
 	}
-	logger.Printf("Info::mdsd::build the datatype and streamid map -- end")	
+	logger.Printf("Info::mdsd::build the datatype and streamid map -- end")
 
 	logger.Printf("extensionconfig::getDataTypeToStreamIdMapping:: getting extension config from fluent socket-end")